diff --git a/CHANGELOG.md b/CHANGELOG.md index d4398313cf8..e2eb4241202 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,10 @@ - Fix typo in `console.warn` regarding fragment matching error message.
[@combizs](https://github.com/combizs) in [#3701](https://github.com/apollographql/apollo-client/pull/3701) +### Apollo Client (vNext) + +- Add proper error handling for subscriptions
+ [@clayne11](https://github.com/clayne11) in [#3800](https://github.com/apollographql/apollo-client/pull/3800) ## 2.3.8 (August 9, 2018) diff --git a/packages/apollo-client/src/__tests__/__snapshots__/graphqlSubscriptions.ts.snap b/packages/apollo-client/src/__tests__/__snapshots__/graphqlSubscriptions.ts.snap new file mode 100644 index 00000000000..5863c7bd92c --- /dev/null +++ b/packages/apollo-client/src/__tests__/__snapshots__/graphqlSubscriptions.ts.snap @@ -0,0 +1,5 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`GraphQL Subscriptions should throw an error if the result has errors on it 1`] = `[Error: GraphQL error: This is an error]`; + +exports[`GraphQL Subscriptions should throw an error if the result has errors on it 2`] = `[Error: GraphQL error: This is an error]`; diff --git a/packages/apollo-client/src/__tests__/graphqlSubscriptions.ts b/packages/apollo-client/src/__tests__/graphqlSubscriptions.ts index 89a70da8aff..a7094ee09cf 100644 --- a/packages/apollo-client/src/__tests__/graphqlSubscriptions.ts +++ b/packages/apollo-client/src/__tests__/graphqlSubscriptions.ts @@ -209,4 +209,53 @@ describe('GraphQL Subscriptions', () => { link.simulateResult(results[0]); }); + + it('should throw an error if the result has errors on it', () => { + const link = mockObservableLink(sub1); + const queryManager = new QueryManager({ + link, + store: new DataStore(new InMemoryCache({ addTypename: false })), + }); + + const obs = queryManager.startGraphQLSubscription(options); + + const promises = []; + for (let i = 0; i < 2; i += 1) { + promises.push( + new Promise((resolve, reject) => { + obs.subscribe({ + next(result) { + fail('Should have hit the error block'); + reject(); + }, + error(error) { + expect(error).toMatchSnapshot(); + resolve(); + }, + }); + }), + ); + } + + const errorResult = { + result: { + data: null, + errors: [ + { + message: 'This is an error', + locations: [ + { + column: 3, + line: 2, + }, + ], + path: ['result'], + }, + ], + }, + }; + + link.simulateResult(errorResult); + return Promise.all(promises); + }); }); diff --git a/packages/apollo-client/src/core/QueryManager.ts b/packages/apollo-client/src/core/QueryManager.ts index 279705b1de1..07958ff4669 100644 --- a/packages/apollo-client/src/core/QueryManager.ts +++ b/packages/apollo-client/src/core/QueryManager.ts @@ -818,11 +818,11 @@ export class QueryManager { const queryName = definition.name ? definition.name.value : null; this.setQuery(queryId, () => ({ observableQuery: null })); if (queryName) { - this.queryIdsByName[queryName] = this.queryIdsByName[ - queryName - ].filter(val => { - return !(observableQuery.queryId === val); - }); + this.queryIdsByName[queryName] = this.queryIdsByName[queryName].filter( + val => { + return !(observableQuery.queryId === val); + }, + ); } } @@ -929,17 +929,29 @@ export class QueryManager { this.broadcastQueries(); } - // It's slightly awkward that the data for subscriptions doesn't - // come from the store. observers.forEach(obs => { - // XXX I'd prefer a different way to handle errors for - // subscriptions. - if (obs.next) obs.next(result); + // If an error exists and a `error` handler has been defined on + // the observer, call that `error` handler and make sure the + // `next` handler is skipped. If no `error` handler exists, we're + // still passing any errors that might occur into the `next` + // handler, to give that handler a chance to deal with the + // error (we're doing this for backwards compatibilty). + if (graphQLResultHasError(result) && obs.error) { + obs.error( + new ApolloError({ + graphQLErrors: result.errors, + }), + ); + } else if (obs.next) { + obs.next(result); + } }); }, error: (error: Error) => { observers.forEach(obs => { - if (obs.error) obs.error(error); + if (obs.error) { + obs.error(error); + } }); }, };