Skip to content

Commit

Permalink
Squash error handling
Browse files Browse the repository at this point in the history
Mutations basic run

ApolloLink

Revert "Mutations basic run"

This reverts commit b3a130d.

Test support

Rename error_network -> errorNetwork

onError link support

bump bundlesize from 29.95 -> 29.98KB

Consolidate errorNetwork away
  • Loading branch information
MrDoomBringer committed Aug 19, 2022
1 parent 888f34f commit 27c85d0
Show file tree
Hide file tree
Showing 10 changed files with 76 additions and 67 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
{
"name": "apollo-client",
"path": "./dist/apollo-client.min.cjs",
"maxSize": "29.95kB"
"maxSize": "29.98kB"
}
],
"engines": {
Expand Down
4 changes: 4 additions & 0 deletions src/cache/core/types/Cache.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { DataProxy } from './DataProxy';
import { Modifier, Modifiers } from './common';
import { ApolloCache } from '../cache';
import { ServerError } from '../../../core';
import { GraphQLError } from 'graphql';

export namespace Cache {
export type WatchCallback<TData = any> = (
Expand All @@ -23,6 +25,8 @@ export namespace Cache {
{
dataId?: string;
result: TResult;
error?: ServerError;
errors?: ReadonlyArray<GraphQLError>;
}

export interface DiffOptions<
Expand Down
2 changes: 2 additions & 0 deletions src/core/QueryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
}

Expand Down
3 changes: 2 additions & 1 deletion src/link/core/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand All @@ -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;
Expand Down
8 changes: 3 additions & 5 deletions src/link/error/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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) {
Expand Down
54 changes: 20 additions & 34 deletions src/link/http/__tests__/HttpLink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1026,15 +1026,18 @@ describe('HttpLink', () => {

describe('Error handling', () => {
let responseBody: any;
const networkError = new Error(`Response not successful: Received status code 400.`) as ServerError;
const text = jest.fn(() => {
const responseBodyText = '{}';
responseBody = JSON.parse(responseBodyText);
responseBody.errorNetwork = networkError
return Promise.resolve(responseBodyText);
});
const textWithData = jest.fn(() => {
responseBody = {
data: { stub: { id: 1 } },
errors: [{ message: 'dangit' }],
errorNetwork: networkError,
};

return Promise.resolve(JSON.stringify(responseBody));
Expand All @@ -1043,6 +1046,7 @@ describe('HttpLink', () => {
const textWithErrors = jest.fn(() => {
responseBody = {
errors: [{ message: 'dangit' }],
errorNetwork: networkError,
};

return Promise.resolve(JSON.stringify(responseBody));
Expand All @@ -1053,18 +1057,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),
});

Expand All @@ -1078,7 +1077,9 @@ describe('HttpLink', () => {

execute(link, { query: sampleQuery }).subscribe(
result => {
reject('next should have been thrown from the network');
expect(result.errorNetwork?.statusCode).toEqual(401);
expect(result.errorNetwork?.message).toMatch(/Received status code 401/);
resolve();
},
() => {},
);
Expand All @@ -1090,13 +1091,11 @@ describe('HttpLink', () => {

execute(link, { query: sampleQuery }).subscribe(
result => {
reject('next should have been thrown from the network');
expect(result.errorNetwork?.statusCode).toBe(400);
expect(result.errorNetwork?.message).toMatch(/Received status code 400/);
expect(result).toEqual(responseBody);
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) => {
Expand All @@ -1106,18 +1105,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.errorNetwork?.statusCode).toBe(400);
expect(result.errorNetwork?.message).toMatch(/Received status code 400/);
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);
resolve();
},
);
Expand All @@ -1131,12 +1123,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);
expect(result.errorNetwork?.statusCode).toEqual(400);
expect(result.errorNetwork?.message).toMatch(/Received status code 400/);
expect(result.errorNetwork?.result).toBe(undefined);
resolve();
},
);
Expand All @@ -1148,13 +1137,10 @@ describe('HttpLink', () => {

execute(link, { query: sampleQuery }).subscribe(
result => {
reject('next should have been thrown from the network');
expect(result.errorNetwork?.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) => {
Expand Down
27 changes: 20 additions & 7 deletions src/link/http/__tests__/parseAndCheckHttpResponse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,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;
Expand All @@ -45,30 +45,43 @@ describe('parseAndCheckResponse', () => {
});
fetch('error')
.then(parseAndCheckHttpResponse(operations))
.then(reject)
.catch(e => {
expect(e.statusCode).toBe(status);
expect(e.name).toBe('ServerError');
.then(({ data, errorNetwork:e }) => {
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) => {
const data = { hello: 'world' }; //does not contain data or erros
const status = 200;
fetchMock.mock('begin:/incorrect', data);
fetch('incorrect')
.then(parseAndCheckHttpResponse(operations))
.then(({ data, errorNetwork:e }) => {
expect(data).toEqual(undefined);
expect(e.name).toEqual("ServerError");
expect(e.statusCode).toEqual(status);
expect(e).toHaveProperty('response');
const message = `Server response was missing for query '${operations.map(op => op.operationName)}'.`;
expect(e.message).toEqual(message)
expect(e.result).toEqual(undefined)
resolve();
})
/*
.then(reject)
.catch(e => {
expect(e.statusCode).toBe(200);
expect(e.name).toBe('ServerError');
expect(e).toHaveProperty('response');
expect(e.result).toEqual(data);
resolve();
})
})*/
.catch(reject);
});

Expand Down
36 changes: 19 additions & 17 deletions src/link/http/parseAndCheckHttpResponse.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { Operation } from '../core';
import { throwServerError } from '../utils';

import { ServerError } from '../utils';
const { hasOwnProperty } = Object.prototype;

export type ServerParseError = Error & {
Expand Down Expand Up @@ -29,28 +28,31 @@ 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;
result.error = 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;
result.error = error;
}
return result;
});
Expand Down
2 changes: 1 addition & 1 deletion src/link/utils/throwServerError.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export type ServerError = Error & {
response: Response;
result: Record<string, any>;
result: Record<string, any> | undefined;
statusCode: number;
};

Expand Down
5 changes: 4 additions & 1 deletion src/react/hooks/useQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 27c85d0

Please sign in to comment.