Makes error handling more consistent

package.json

@@ -56,7 +56,7 @@
     {
       "name": "apollo-client",
       "path": "./dist/apollo-client.min.cjs",
-      "maxSize": "29.95kB"
+      "maxSize": "30.01kB"
     }
   ],
   "engines": {

src/core/QueryManager.ts

@@ -1047,7 +1047,7 @@ export class QueryManager<TStore> {
       ),
 
       result => {
-        const hasErrors = isNonEmptyArray(result.errors);
+        const hasErrors = isNonEmptyArray(result.errors) || result.error;
 
         // If we interrupted this request by calling getResultsFromLink again
         // with the same QueryInfo object, we ignore the old results.
@@ -1056,6 +1056,7 @@ export class QueryManager<TStore> {
             // Throwing here effectively calls observer.error.
             throw queryInfo.markError(new ApolloError({
               graphQLErrors: result.errors,
+              networkError: result.error,
             }));
           }
           queryInfo.markResult(result, options, cacheWriteBehavior);
@@ -1070,6 +1071,7 @@ export class QueryManager<TStore> {
 
         if (hasErrors && options.errorPolicy !== "ignore") {
           aqr.errors = result.errors;
+          aqr.error = result.error;
           aqr.networkStatus = NetworkStatus.error;
         }
 

src/core/__tests__/ObservableQuery.ts

@@ -1860,9 +1860,11 @@ describe('ObservableQuery', () => {
     });
 
     itAsync('returns errors with data if errorPolicy is all', (resolve, reject) => {
+      const networkError = new ApolloError({errorMessage: "Oh no"})
+
       const queryManager = mockQueryManager(reject, {
         request: { query, variables },
-        result: { data: dataOne, errors: [error] },
+        result: { data: dataOne, errors: [error], error: networkError},
       });
 
       const observable = queryManager.watchQuery({
@@ -1877,7 +1879,7 @@ describe('ObservableQuery', () => {
         const currentResult = observable.getCurrentResult();
         expect(currentResult.loading).toBe(false);
         expect(currentResult.errors).toEqual([error]);
-        expect(currentResult.error).toBeUndefined();
+        expect(currentResult.error).toEqual(networkError);
       }).then(resolve, reject);
     });
 

src/link/core/types.ts

@@ -2,7 +2,7 @@ import { DocumentNode, ExecutionResult } from 'graphql';
 export { DocumentNode };
 
 import { Observable } from '../../utilities';
-
+import { ApolloError } from '../../errors';
 export interface GraphQLRequest {
   query: DocumentNode;
   variables?: Record<string, any>;
@@ -25,6 +25,7 @@ export interface FetchResult<
   TContext = Record<string, any>,
   TExtensions = Record<string, any>
 > extends ExecutionResult<TData, TExtensions> {
+  error?: ApolloError;
   data?: TData | null | undefined;
   extensions?: TExtensions;
   context?: TContext;

src/link/error/index.ts

@@ -34,9 +34,10 @@ export function onError(errorHandler: ErrorHandler): ApolloLink {
       try {
         sub = forward(operation).subscribe({
           next: result => {
-            if (result.errors) {
+            if (result.errors || result.error) {
               retriedResult = errorHandler({
                 graphQLErrors: result.errors,
+                networkError: result.error,
                 response: result,
                 operation,
                 forward,
@@ -58,10 +59,7 @@ export function onError(errorHandler: ErrorHandler): ApolloLink {
               operation,
               networkError,
               //Network errors can return GraphQL errors on for example a 403
-              graphQLErrors:
-                networkError &&
-                networkError.result &&
-                networkError.result.errors,
+              graphQLErrors: networkError?.result?.errors,
               forward,
             });
             if (retriedResult) {

src/link/http/__tests__/HttpLink.ts

@@ -1053,18 +1053,13 @@ describe('HttpLink', () => {
     beforeEach(() => {
       fetch.mockReset();
     });
-    itAsync('makes it easy to do stuff on a 401', (resolve, reject) => {
+    itAsync('observables continue after a 401 error', (resolve, reject) => {
       const middleware = new ApolloLink((operation, forward) => {
         return new Observable(ob => {
           fetch.mockReturnValueOnce(Promise.resolve({ status: 401, text }));
           const op = forward(operation);
           const sub = op.subscribe({
             next: ob.next.bind(ob),
-            error: makeCallback(resolve, reject, (e: ServerError) => {
-              expect(e.message).toMatch(/Received status code 401/);
-              expect(e.statusCode).toEqual(401);
-              ob.error(e);
-            }),
             complete: ob.complete.bind(ob),
           });
 
@@ -1078,7 +1073,10 @@ describe('HttpLink', () => {
 
       execute(link, { query: sampleQuery }).subscribe(
         result => {
-          reject('next should have been thrown from the network');
+          const error = result.error?.networkError as ServerError
+          expect(error.statusCode).toEqual(401);
+          expect(error.message).toMatch(/Received status code 401/);
+          resolve();
         },
         () => {},
       );
@@ -1090,13 +1088,12 @@ describe('HttpLink', () => {
 
       execute(link, { query: sampleQuery }).subscribe(
         result => {
-          reject('next should have been thrown from the network');
+          const error = result.error?.networkError as ServerError
+          expect(error.statusCode).toBe(400);
+          expect(error.message).toMatch(/Received status code 400/);
+
+          resolve();
         },
-        makeCallback(resolve, reject, (e: ServerError) => {
-          expect(e.message).toMatch(/Received status code 400/);
-          expect(e.statusCode).toBe(400);
-          expect(e.result).toEqual(responseBody);
-        }),
       );
     });
     itAsync('throws an error if response code is > 300 and returns data', (resolve, reject) => {
@@ -1106,18 +1103,11 @@ describe('HttpLink', () => {
 
       const link = createHttpLink({ uri: 'data', fetch: fetch as any });
 
-      let called = false;
-
       execute(link, { query: sampleQuery }).subscribe(
         result => {
-          called = true;
-          expect(result).toEqual(responseBody);
-        },
-        e => {
-          expect(called).toBe(true);
-          expect(e.message).toMatch(/Received status code 400/);
-          expect(e.statusCode).toBe(400);
-          expect(e.result).toEqual(responseBody);
+          const error = result.error?.networkError as ServerError
+          expect(error.statusCode).toBe(400);
+          expect(error.message).toMatch(/Received status code 400/);
           resolve();
         },
       );
@@ -1131,12 +1121,9 @@ describe('HttpLink', () => {
 
       execute(link, { query: sampleQuery }).subscribe(
         result => {
-          reject('should not have called result because we have no data');
-        },
-        e => {
-          expect(e.message).toMatch(/Received status code 400/);
-          expect(e.statusCode).toBe(400);
-          expect(e.result).toEqual(responseBody);
+          const error = result.error?.networkError as ServerError
+          expect(error.statusCode).toEqual(400);
+          expect(error.message).toMatch(/Received status code 400/);
           resolve();
         },
       );
@@ -1148,13 +1135,11 @@ describe('HttpLink', () => {
 
       execute(link, { query: sampleQuery }).subscribe(
         result => {
-          reject('next should have been thrown from the network');
+          const error = result.error?.networkError as ServerError
+          expect(error.message).toMatch(/Server response was missing for query 'SampleQuery'./);
+          resolve();
         },
-        makeCallback(resolve, reject, (e: Error) => {
-          expect(e.message).toMatch(
-            /Server response was missing for query 'SampleQuery'/,
-          );
-        }),
+
       );
     });
     itAsync("throws if the body can't be stringified", (resolve, reject) => {

src/link/http/__tests__/parseAndCheckHttpResponse.ts

@@ -4,6 +4,7 @@ import fetchMock from 'fetch-mock';
 import { createOperation } from '../../utils/createOperation';
 import { parseAndCheckHttpResponse } from '../parseAndCheckHttpResponse';
 import { itAsync } from '../../../testing';
+import { ServerError } from '../../utils';
 
 const query = gql`
   query SampleQuery {
@@ -18,7 +19,7 @@ describe('parseAndCheckResponse', () => {
     fetchMock.restore();
   });
 
-  const operations = [createOperation({}, { query })];
+  const operations = [createOperation({}, { query, operationName:"SampleQuery" })];
 
   itAsync('throws a parse error with a status code on unparsable response', (resolve, reject) => {
     const status = 400;
@@ -36,7 +37,7 @@ describe('parseAndCheckResponse', () => {
       .catch(reject);
   });
 
-  itAsync('throws a network error with a status code and result', (resolve, reject) => {
+  itAsync('returns a network error with a status code and result', (resolve, reject) => {
     const status = 403;
     const body = { data: 'fail' }; //does not contain data or errors
     fetchMock.mock('begin:/error', {
@@ -45,28 +46,34 @@ describe('parseAndCheckResponse', () => {
     });
     fetch('error')
       .then(parseAndCheckHttpResponse(operations))
-      .then(reject)
-      .catch(e => {
-        expect(e.statusCode).toBe(status);
-        expect(e.name).toBe('ServerError');
+      .then(({ data, error }) => {
+        const e = error.networkError as ServerError
+        expect(data).toEqual('fail');
+        expect(e.name).toEqual("ServerError");
+        expect(e.statusCode).toEqual(status);
         expect(e).toHaveProperty('response');
-        expect(e).toHaveProperty('result');
+        expect(e.message).toEqual(`Response not successful: Received status code ${status}.`)
+        expect(e.result).toEqual(undefined)
         resolve();
       })
       .catch(reject);
   });
 
-  itAsync('throws a server error on incorrect data', (resolve, reject) => {
+  itAsync('returns a server error on incorrect data', (resolve, reject) => {
     const data = { hello: 'world' }; //does not contain data or erros
+    const status = 200;
     fetchMock.mock('begin:/incorrect', data);
     fetch('incorrect')
       .then(parseAndCheckHttpResponse(operations))
-      .then(reject)
-      .catch(e => {
-        expect(e.statusCode).toBe(200);
-        expect(e.name).toBe('ServerError');
+      .then(({ data, error }) => {
+        const e = error.networkError as ServerError
+        expect(data).toEqual(undefined);
+        expect(e.name).toEqual("ServerError");
+        expect(e.statusCode).toEqual(status);
         expect(e).toHaveProperty('response');
-        expect(e.result).toEqual(data);
+        const message = `Server response was missing for query '${operations.map(op => op.operationName)}'.`;
+        expect(e.message).toEqual(message)
+        expect(e.result).toEqual(undefined)
         resolve();
       })
       .catch(reject);

src/link/http/parseAndCheckHttpResponse.ts

@@ -1,6 +1,6 @@
+import { ApolloError } from '../../errors';
 import { Operation } from '../core';
-import { throwServerError } from '../utils';
-
+import { ServerError } from '../utils';
 const { hasOwnProperty } = Object.prototype;
 
 export type ServerParseError = Error & {
@@ -29,28 +29,39 @@ export function parseAndCheckHttpResponse(
     .then((result: any) => {
       if (response.status >= 300) {
         // Network error
-        throwServerError(
-          response,
-          result,
-          `Response not successful: Received status code ${response.status}`,
-        );
+        const message = `Response not successful: Received status code ${response.status}.`;
+        const error = new Error(message) as ServerError;
+        error.name = 'ServerError';
+        error.response = response;
+        error.statusCode = response.status;
+        if (result.error) {
+          result.error.networkError = error;
+        } else {
+          result.error = new ApolloError({networkError: error});
+        }
+        return result;
       }
 
       if (
         !Array.isArray(result) &&
         !hasOwnProperty.call(result, 'data') &&
         !hasOwnProperty.call(result, 'errors')
       ) {
-        // Data error
-        throwServerError(
-          response,
-          result,
-          `Server response was missing for query '${
-            Array.isArray(operations)
-              ? operations.map(op => op.operationName)
-              : operations.operationName
-          }'.`,
-        );
+        // Data error (Missing a useful response from the server)
+        const message = `Server response was missing for query '${
+          Array.isArray(operations)
+            ? operations.map(op => op.operationName)
+            : operations.operationName
+        }'.`;
+        const error = new Error(message) as ServerError;
+        error.name = 'ServerError';
+        error.response = response;
+        error.statusCode = response.status;
+        if (result.error) {
+          result.error.networkError = error;
+        } else {
+          result.error = new ApolloError({networkError: error});
+        }  
       }
       return result;
     });

src/link/utils/throwServerError.ts

@@ -1,6 +1,6 @@
 export type ServerError = Error & {
   response: Response;
-  result: Record<string, any>;
+  result: Record<string, any> | undefined;
   statusCode: number;
 };
 

src/react/hooks/useQuery.ts

@@ -560,7 +560,10 @@ class InternalState<TData, TVariables> {
       // decided upon, we map errors (graphQLErrors) to the error options.
       // TODO: Is it possible for both result.error and result.errors to be
       // defined here?
-      queryResult.error = new ApolloError({ graphQLErrors: result.errors });
+      queryResult.error = new ApolloError({ 
+        graphQLErrors: result.errors,
+        networkError: result.error,
+       });
     }
 
     return queryResult;