From d1ce23eba19f3326e4e381e82f0888fc0e97eb66 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 9 Oct 2020 17:26:13 -0400 Subject: [PATCH] Let mockLink.onError return false to prevent observer.error. https://github.com/apollographql/apollo-client/pull/7110#discussion_r500419910 --- src/link/core/ApolloLink.ts | 22 ++++++++++--- src/utilities/testing/mocking/mockLink.ts | 38 +++++++++++++---------- 2 files changed, 39 insertions(+), 21 deletions(-) diff --git a/src/link/core/ApolloLink.ts b/src/link/core/ApolloLink.ts index f9dce653aa2..2cc0aa8e26f 100644 --- a/src/link/core/ApolloLink.ts +++ b/src/link/core/ApolloLink.ts @@ -141,13 +141,25 @@ export class ApolloLink { throw new InvariantError('request is not implemented'); } - protected onError(error: any, observer: ZenObservable.Observer) { - observer.error!(error); + protected onError( + error: any, + observer: ZenObservable.Observer, + ): false | void { + if (observer && observer.error) { + observer.error(error); + // Returning false indicates that observer.error does not need to be + // called again, since it was already called (on the previous line). + // Calling observer.error again would not cause any real problems, + // since only the first call matters, but custom onError functions + // might have other reasons for wanting to prevent the default + // behavior by returning false. + return false; + } + // Throw errors will be passed to observer.error. + throw error; } - public setOnError( - fn: (error: any, observer?: ZenObservable.Observer) => any - ): this { + public setOnError(fn: ApolloLink["onError"]): this { this.onError = fn; return this; } diff --git a/src/utilities/testing/mocking/mockLink.ts b/src/utilities/testing/mocking/mockLink.ts index 8456147a806..a754c3640bc 100644 --- a/src/utilities/testing/mocking/mockLink.ts +++ b/src/utilities/testing/mocking/mockLink.ts @@ -110,29 +110,35 @@ export class MockLink extends ApolloLink { } return new Observable(observer => { - let timer = setTimeout( - () => { - if (configError) { - try { - this.onError(configError, observer); - } catch (error) { - observer.error(error); + const timer = setTimeout(() => { + if (configError) { + try { + // The onError function can return false to indicate that + // configError need not be passed to observer.error. For + // example, the default implementation of onError calls + // observer.error(configError) and then returns false to + // prevent this extra (harmless) observer.error call. + if (this.onError(configError, observer) !== false) { + throw configError; } - } else if (response!.error) { - observer.error(response!.error); + } catch (error) { + observer.error(error); + } + } else if (response) { + if (response.error) { + observer.error(response.error); } else { - if (response!.result) { + if (response.result) { observer.next( - typeof response!.result === 'function' - ? (response!.result as ResultFunction)() - : response!.result + typeof response.result === 'function' + ? (response.result as ResultFunction)() + : response.result ); } observer.complete(); } - }, - response?.delay || 0 - ); + } + }, response && response.delay || 0); return () => { clearTimeout(timer);