From ebf378637aaaeebcf25062b3ecb2b77bf0510f33 Mon Sep 17 00:00:00 2001
From: Farhan Attamimi <farhan@sourcegraph.com>
Date: Tue, 30 Oct 2018 16:22:19 -0700
Subject: [PATCH 1/3] fix: error emitted from a hover provider broke other
 hover providers

---
 src/client/providers/hover.ts | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/src/client/providers/hover.ts b/src/client/providers/hover.ts
index 0743a0e..549c47b 100644
--- a/src/client/providers/hover.ts
+++ b/src/client/providers/hover.ts
@@ -1,5 +1,5 @@
 import { combineLatest, from, Observable } from 'rxjs'
-import { map, switchMap } from 'rxjs/operators'
+import { map, switchMap, catchError } from 'rxjs/operators'
 import { HoverMerged } from '../../client/types/hover'
 import { TextDocumentPositionParams, TextDocumentRegistrationOptions } from '../../protocol'
 import { Hover } from '../../protocol/plainTypes'
@@ -14,6 +14,11 @@ export class TextDocumentHoverProviderRegistry extends FeatureProviderRegistry<
     TextDocumentRegistrationOptions,
     ProvideTextDocumentHoverSignature
 > {
+    /**
+     * Returns an observable that emits all providers' hovers whenever any of the last-emitted set of providers emits
+     * hovers. If any provider emits an error, the error is logged and the provider is omitted from the emission of
+     * the observable (the observable does not emit the error).
+     */
     public getHover(params: TextDocumentPositionParams): Observable<HoverMerged | null> {
         return getHover(this.providers, params)
     }
@@ -21,7 +26,8 @@ export class TextDocumentHoverProviderRegistry extends FeatureProviderRegistry<
 
 /**
  * Returns an observable that emits all providers' hovers whenever any of the last-emitted set of providers emits
- * hovers.
+ * hovers. If any provider emits an error, the error is logged and the provider is omitted from the emission of
+ * the observable (the observable does not emit the error).
  *
  * Most callers should use TextDocumentHoverProviderRegistry's getHover method, which uses the registered hover
  * providers.
@@ -36,7 +42,18 @@ export function getHover(
                 if (providers.length === 0) {
                     return [[null]]
                 }
-                return combineLatest(providers.map(provider => from(provider(params))))
+                return combineLatest(
+                    providers.map(provider =>
+                        from(
+                            provider(params).pipe(
+                                catchError(err => {
+                                    console.error(err)
+                                    return [null]
+                                })
+                            )
+                        )
+                    )
+                )
             })
         )
         .pipe(map(HoverMerged.from))

From 863b1cef721c56e8c5d3925f03274a5a0c21fe4f Mon Sep 17 00:00:00 2001
From: Farhan Attamimi <farhan@sourcegraph.com>
Date: Tue, 30 Oct 2018 17:07:34 -0700
Subject: [PATCH 2/3] fix: tslint

---
 src/client/providers/hover.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/client/providers/hover.ts b/src/client/providers/hover.ts
index 549c47b..bcc4ef4 100644
--- a/src/client/providers/hover.ts
+++ b/src/client/providers/hover.ts
@@ -1,5 +1,5 @@
 import { combineLatest, from, Observable } from 'rxjs'
-import { map, switchMap, catchError } from 'rxjs/operators'
+import { catchError, map, switchMap } from 'rxjs/operators'
 import { HoverMerged } from '../../client/types/hover'
 import { TextDocumentPositionParams, TextDocumentRegistrationOptions } from '../../protocol'
 import { Hover } from '../../protocol/plainTypes'

From f2c6e8c46a75eb79aae6c2e007ceaec0c5bc0ace Mon Sep 17 00:00:00 2001
From: Farhan Attamimi <farhan@sourcegraph.com>
Date: Tue, 30 Oct 2018 17:47:23 -0700
Subject: [PATCH 3/3] chore: add test for error in hover provider

---
 src/client/providers/hover.test.ts | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/src/client/providers/hover.test.ts b/src/client/providers/hover.test.ts
index d46b665..af8b050 100644
--- a/src/client/providers/hover.test.ts
+++ b/src/client/providers/hover.test.ts
@@ -1,5 +1,5 @@
 import * as assert from 'assert'
-import { of } from 'rxjs'
+import { of, throwError } from 'rxjs'
 import { TestScheduler } from 'rxjs/testing'
 import { Hover, MarkupKind } from 'sourcegraph'
 import { HoverMerged } from '../../client/types/hover'
@@ -83,6 +83,20 @@ describe('getHover', () => {
                 })
             ))
 
+        it('omits error result from 1 provider', () =>
+            scheduler().run(({ cold, expectObservable }) =>
+                expectObservable(
+                    getHover(
+                        cold<ProvideTextDocumentHoverSignature[]>('-a-|', {
+                            a: [() => throwError('err'), () => of(FIXTURE_RESULT)],
+                        }),
+                        FIXTURE.TextDocumentPositionParams
+                    )
+                ).toBe('-a-|', {
+                    a: FIXTURE_RESULT_MERGED,
+                })
+            ))
+
         it('merges results from providers', () =>
             scheduler().run(({ cold, expectObservable }) =>
                 expectObservable(