Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove "[GraphQL|Network] error" from error messages #3892

Merged
merged 1 commit into from
Apr 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@
- **[BREAKING]** Apollo Client will no longer deliver "stale" results to `ObservableQuery` consumers, but will instead log more helpful errors about which cache fields were missing. <br/>
[@benjamn](https://github.com/benjamn) in [#6058](https://github.com/apollographql/apollo-client/pull/6058)

- **[BREAKING]** `ApolloError`'s thrown by Apollo Client no longer prefix error messages with `GraphQL error:` or `Network error:`. To differentiate between GraphQL/network errors, refer to `ApolloError`'s public `graphQLErrors` and `networkError` properties. <br/>
[@lorensr](https://github.com/lorensr) in [#3892](https://github.com/apollographql/apollo-client/pull/3892)

- `InMemoryCache` now supports tracing garbage collection and eviction. Note that the signature of the `evict` method has been simplified in a potentially backwards-incompatible way. <br/>
[@benjamn](https://github.com/benjamn) in [#5310](https://github.com/apollographql/apollo-client/pull/5310)

Expand Down
4 changes: 2 additions & 2 deletions src/__tests__/__snapshots__/graphqlSubscriptions.ts.snap
Original file line number Diff line number Diff line change
@@ -1,5 +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 1`] = `[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]`;
exports[`GraphQL Subscriptions should throw an error if the result has errors on it 2`] = `[Error: This is an error]`;
6 changes: 3 additions & 3 deletions src/__tests__/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2388,7 +2388,7 @@ describe('client', () => {

handle.subscribe({
error(error) {
expect(error.message).toBe('Network error: Uh oh!');
expect(error.message).toBe('Uh oh!');
resolve();
},
});
Expand Down Expand Up @@ -2483,7 +2483,7 @@ describe('client', () => {
},
error(error) {
expect(count++).toBe(2);
expect(error.message).toBe('Network error: This is an error!');
expect(error.message).toBe('This is an error!');

subscription.unsubscribe();

Expand Down Expand Up @@ -2536,7 +2536,7 @@ describe('client', () => {

return client.query({ query }).catch(err => {
expect(err.message).toBe(
'GraphQL error: Cannot query field "foo" on type "Post".',
'Cannot query field "foo" on type "Post".',
);
}).then(resolve, reject);
});
Expand Down
4 changes: 2 additions & 2 deletions src/__tests__/local-state/__snapshots__/general.ts.snap
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Combining client and server state/operations should correctly propagate an error from a client resolver 1`] = `"Network error: Illegal Query Operation Occurred"`;
exports[`Combining client and server state/operations should correctly propagate an error from a client resolver 1`] = `"Illegal Query Operation Occurred"`;

exports[`Combining client and server state/operations should correctly propagate an error from a client resolver 2`] = `"Network error: Illegal Mutation Operation Occurred"`;
exports[`Combining client and server state/operations should correctly propagate an error from a client resolver 2`] = `"Illegal Mutation Operation Occurred"`;
12 changes: 6 additions & 6 deletions src/__tests__/optimistic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ describe('optimistic mutation results', () => {
await promise;
} catch (err) {
expect(err).toBeInstanceOf(Error);
expect(err.message).toBe('Network error: forbidden (test error)');
expect(err.message).toBe('forbidden (test error)');

const dataInStore = (client.cache as InMemoryCache).extract(true);
expect((dataInStore['TodoList5'] as any).todos.length).toBe(3);
Expand Down Expand Up @@ -254,7 +254,7 @@ describe('optimistic mutation results', () => {
.catch(err => {
// it is ok to fail here
expect(err).toBeInstanceOf(Error);
expect(err.message).toBe('Network error: forbidden (test error)');
expect(err.message).toBe('forbidden (test error)');
return null;
});

Expand Down Expand Up @@ -447,7 +447,7 @@ describe('optimistic mutation results', () => {
await promise;
} catch (err) {
expect(err).toBeInstanceOf(Error);
expect(err.message).toBe('Network error: forbidden (test error)');
expect(err.message).toBe('forbidden (test error)');

const dataInStore = (client.cache as InMemoryCache).extract(true);
expect((dataInStore['TodoList5'] as any).todos.length).toBe(3);
Expand Down Expand Up @@ -491,7 +491,7 @@ describe('optimistic mutation results', () => {
.catch(err => {
// it is ok to fail here
expect(err).toBeInstanceOf(Error);
expect(err.message).toBe('Network error: forbidden (test error)');
expect(err.message).toBe('forbidden (test error)');
return null;
});

Expand Down Expand Up @@ -1193,7 +1193,7 @@ describe('optimistic mutation results', () => {
.catch(err => {
// it is ok to fail here
expect(err).toBeInstanceOf(Error);
expect(err.message).toEqual('Network error: forbidden (test error)');
expect(err.message).toEqual('forbidden (test error)');
return null;
});

Expand Down Expand Up @@ -1654,7 +1654,7 @@ describe('optimistic mutation results', () => {
.catch(err => {
// it is ok to fail here
expect(err).toBeInstanceOf(Error);
expect(err.message).toBe('Network error: forbidden (test error)');
expect(err.message).toBe('forbidden (test error)');
return null;
});

Expand Down
2 changes: 1 addition & 1 deletion src/core/__tests__/QueryManager/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ describe('QueryManager', () => {
},
error(error) {
expect((error as any).graphQLErrors).toEqual([null]);
expect(error.message).toBe('GraphQL error: Error message not found.');
expect(error.message).toBe('Error message not found.');
resolve();
},
},
Expand Down
4 changes: 2 additions & 2 deletions src/errors/ApolloError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@ const generateErrorMessage = (err: ApolloError) => {
const errorMessage = graphQLError
? graphQLError.message
: 'Error message not found.';
message += `GraphQL error: ${errorMessage}\n`;
message += `${errorMessage}\n`;
});
}

if (err.networkError) {
message += 'Network error: ' + err.networkError.message + '\n';
message += `${err.networkError.message}\n`;
}

// strip newline from the end of the message
Expand Down
6 changes: 0 additions & 6 deletions src/errors/__tests__/ApolloError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ describe('ApolloError', () => {
const apolloError = new ApolloError({
networkError,
});
expect(apolloError.message).toMatch('Network error: ');
expect(apolloError.message).toMatch('this is an error message');
expect(apolloError.message.split('\n').length).toBe(1);
});
Expand All @@ -33,7 +32,6 @@ describe('ApolloError', () => {
const apolloError = new ApolloError({
graphQLErrors,
});
expect(apolloError.message).toMatch('GraphQL error: ');
expect(apolloError.message).toMatch('this is an error message');
expect(apolloError.message.split('\n').length).toBe(1);
});
Expand All @@ -45,9 +43,7 @@ describe('ApolloError', () => {
});
const messages = apolloError.message.split('\n');
expect(messages.length).toBe(2);
expect(messages[0]).toMatch('GraphQL error');
expect(messages[0]).toMatch('this is new');
expect(messages[1]).toMatch('GraphQL error');
expect(messages[1]).toMatch('this is old');
});

Expand All @@ -60,9 +56,7 @@ describe('ApolloError', () => {
});
const messages = apolloError.message.split('\n');
expect(messages.length).toBe(2);
expect(messages[0]).toMatch('GraphQL error');
expect(messages[0]).toMatch('graphql error message');
expect(messages[1]).toMatch('Network error');
expect(messages[1]).toMatch('network error message');
});

Expand Down
24 changes: 12 additions & 12 deletions src/react/hooks/__tests__/useQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ describe('useQuery Hook', () => {
const { loading, error } = useQuery(query);
if (!loading) {
expect(error).toBeDefined();
expect(error!.message).toEqual('GraphQL error: forced error');
expect(error!.message).toEqual('forced error');
}
return null;
};
Expand Down Expand Up @@ -594,7 +594,7 @@ describe('useQuery Hook', () => {
case 2:
expect(loading).toBeFalsy();
expect(error).toBeDefined();
expect(error!.message).toEqual('Network error: Oh no!');
expect(error!.message).toEqual('Oh no!');
onErrorPromise.then(() => refetch());
break;
case 3:
Expand Down Expand Up @@ -648,14 +648,14 @@ describe('useQuery Hook', () => {
break;
case 1:
expect(error).toBeDefined();
expect(error!.message).toEqual('GraphQL error: forced error');
expect(error!.message).toEqual('forced error');
setTimeout(() => {
forceUpdate(0);
});
break;
case 2:
expect(error).toBeDefined();
expect(error!.message).toEqual('GraphQL error: forced error');
expect(error!.message).toEqual('forced error');
break;
default: // Do nothing
}
Expand Down Expand Up @@ -711,14 +711,14 @@ describe('useQuery Hook', () => {
break;
case 1:
expect(error).toBeDefined();
expect(error!.message).toEqual('GraphQL error: forced error');
expect(error!.message).toEqual('forced error');
setTimeout(() => {
forceUpdate(0);
});
break;
case 2:
expect(error).toBeDefined();
expect(error!.message).toEqual('GraphQL error: forced error');
expect(error!.message).toEqual('forced error');
break;
default: // Do nothing
}
Expand Down Expand Up @@ -777,7 +777,7 @@ describe('useQuery Hook', () => {
case 2:
expect(loading).toBeFalsy();
expect(error).toBeDefined();
expect(error!.message).toEqual('GraphQL error: an error 1');
expect(error!.message).toEqual('an error 1');
setTimeout(() => {
// catch here to avoid failing due to 'uncaught promise rejection'
refetch().catch(() => {});
Expand All @@ -786,7 +786,7 @@ describe('useQuery Hook', () => {
case 3:
expect(loading).toBeFalsy();
expect(error).toBeDefined();
expect(error!.message).toEqual('GraphQL error: an error 2');
expect(error!.message).toEqual('an error 2');
break;
default: // Do nothing
}
Expand Down Expand Up @@ -843,9 +843,9 @@ describe('useQuery Hook', () => {
case 2:
expect(loading).toBeFalsy();
expect(error).toBeDefined();
expect(error!.message).toEqual('GraphQL error: same error message');
expect(error!.message).toEqual('same error message');
refetch().catch(error => {
if (error.message !== 'GraphQL error: same error message') {
if (error.message !== 'same error message') {
reject(error);
}
});
Expand Down Expand Up @@ -903,7 +903,7 @@ describe('useQuery Hook', () => {
case 2:
expect(loading).toBeFalsy();
expect(error).toBeDefined();
expect(error!.message).toEqual('GraphQL error: same error message');
expect(error!.message).toEqual('same error message');
setTimeout(() => {
// catch here to avoid failing due to 'uncaught promise rejection'
refetch().catch(() => {});
Expand All @@ -924,7 +924,7 @@ describe('useQuery Hook', () => {
case 5:
expect(loading).toBeFalsy();
expect(error).toBeDefined();
expect(error!.message).toEqual('GraphQL error: same error message');
expect(error!.message).toEqual('same error message');
break;
default: // Do nothing
}
Expand Down