Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
This repository was archived by the owner on Nov 6, 2018. It is now read-only.

Commit f7a6d85

Browse filesBrowse files
authored
fix: error emitted from one hover provider would break other hover providers (#117)
1 parent 6126d5f commit f7a6d85
Copy full SHA for f7a6d85

2 files changed

+35-4Lines changed: 35 additions & 4 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎src/client/providers/hover.test.ts‎

Copy file name to clipboardExpand all lines: src/client/providers/hover.test.ts
+15-1Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as assert from 'assert'
2-
import { of } from 'rxjs'
2+
import { of, throwError } from 'rxjs'
33
import { TestScheduler } from 'rxjs/testing'
44
import { Hover, MarkupKind } from 'sourcegraph'
55
import { HoverMerged } from '../../client/types/hover'
@@ -83,6 +83,20 @@ describe('getHover', () => {
8383
})
8484
))
8585

86+
it('omits error result from 1 provider', () =>
87+
scheduler().run(({ cold, expectObservable }) =>
88+
expectObservable(
89+
getHover(
90+
cold<ProvideTextDocumentHoverSignature[]>('-a-|', {
91+
a: [() => throwError('err'), () => of(FIXTURE_RESULT)],
92+
}),
93+
FIXTURE.TextDocumentPositionParams
94+
)
95+
).toBe('-a-|', {
96+
a: FIXTURE_RESULT_MERGED,
97+
})
98+
))
99+
86100
it('merges results from providers', () =>
87101
scheduler().run(({ cold, expectObservable }) =>
88102
expectObservable(
Collapse file

‎src/client/providers/hover.ts‎

Copy file name to clipboardExpand all lines: src/client/providers/hover.ts
+20-3Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { combineLatest, from, Observable } from 'rxjs'
2-
import { map, switchMap } from 'rxjs/operators'
2+
import { catchError, map, switchMap } from 'rxjs/operators'
33
import { HoverMerged } from '../../client/types/hover'
44
import { TextDocumentPositionParams, TextDocumentRegistrationOptions } from '../../protocol'
55
import { Hover } from '../../protocol/plainTypes'
@@ -14,14 +14,20 @@ export class TextDocumentHoverProviderRegistry extends FeatureProviderRegistry<
1414
TextDocumentRegistrationOptions,
1515
ProvideTextDocumentHoverSignature
1616
> {
17+
/**
18+
* Returns an observable that emits all providers' hovers whenever any of the last-emitted set of providers emits
19+
* hovers. If any provider emits an error, the error is logged and the provider is omitted from the emission of
20+
* the observable (the observable does not emit the error).
21+
*/
1722
public getHover(params: TextDocumentPositionParams): Observable<HoverMerged | null> {
1823
return getHover(this.providers, params)
1924
}
2025
}
2126

2227
/**
2328
* Returns an observable that emits all providers' hovers whenever any of the last-emitted set of providers emits
24-
* hovers.
29+
* hovers. If any provider emits an error, the error is logged and the provider is omitted from the emission of
30+
* the observable (the observable does not emit the error).
2531
*
2632
* Most callers should use TextDocumentHoverProviderRegistry's getHover method, which uses the registered hover
2733
* providers.
@@ -36,7 +42,18 @@ export function getHover(
3642
if (providers.length === 0) {
3743
return [[null]]
3844
}
39-
return combineLatest(providers.map(provider => from(provider(params))))
45+
return combineLatest(
46+
providers.map(provider =>
47+
from(
48+
provider(params).pipe(
49+
catchError(err => {
50+
console.error(err)
51+
return [null]
52+
})
53+
)
54+
)
55+
)
56+
)
4057
})
4158
)
4259
.pipe(map(HoverMerged.from))

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.