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

Log non-fatal error when fields are missing from written results #8416

Merged
merged 4 commits into from
Jun 22, 2021
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 @@ -44,6 +44,9 @@
- The TypeScript return types of the `getLastResult` and `getLastError` methods of `ObservableQuery` now correctly include the possibility of returning `undefined`. If you happen to be calling either of these methods directly, you may need to adjust how the calling code handles the methods' possibly-`undefined` results. <br/>
[@benjamn](https://github.com/benjamn) in [#8394](https://github.com/apollographql/apollo-client/pull/8394)

- Log non-fatal `invariant.error` message when fields are missing from result objects written into `InMemoryCache`, rather than throwing an exception. While this change relaxes an exception to be merely an error message, which is usually a backwards-compatible change, the error messages are logged in more cases now than the exception was previously thrown, and those new error messages may be worth investigating to discover potential problems in your application. The errors are not displayed for `@client`-only fields, so adding `@client` is one way to handle/hide the errors for local-only fields. Another general strategy is to use a more precise query to write specific subsets of data into the cache, rather than reusing a larger query that contains fields not present in the written `data`. <br/>
[@benjamn](https://github.com/benjamn) in [#8416](https://github.com/apollographql/apollo-client/pull/8416)

### Improvements

- `InMemoryCache` now _guarantees_ that any two result objects returned by the cache (from `readQuery`, `readFragment`, etc.) will be referentially equal (`===`) if they are deeply equal. Previously, `===` equality was often achievable for results for the same query, on a best-effort basis. Now, equivalent result objects will be automatically shared among the result trees of completely different queries. This guarantee is important for taking full advantage of optimistic updates that correctly guess the final data, and for "pure" UI components that can skip re-rendering when their input data are unchanged. <br/>
Expand Down
68 changes: 32 additions & 36 deletions src/__tests__/ApolloClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { Observable } from '../utilities';
import { ApolloLink } from '../link/core';
import { HttpLink } from '../link/http';
import { InMemoryCache } from '../cache';
import { stripSymbols } from '../testing';
import { stripSymbols, withErrorSpy } from '../testing';
import { TypedDocumentNode } from '@graphql-typed-document-node/core';

describe('ApolloClient', () => {
Expand Down Expand Up @@ -834,7 +834,7 @@ describe('ApolloClient', () => {
});
});

it('should warn when the data provided does not match the query shape', () => {
withErrorSpy(it, 'should warn when the data provided does not match the query shape', () => {
const client = new ApolloClient({
link: ApolloLink.empty(),
cache: new InMemoryCache({
Expand All @@ -843,28 +843,26 @@ describe('ApolloClient', () => {
}),
});

expect(() => {
client.writeQuery({
data: {
todos: [
{
id: '1',
name: 'Todo 1',
__typename: 'Todo',
},
],
},
query: gql`
query {
todos {
id
name
description
}
client.writeQuery({
data: {
todos: [
{
id: '1',
name: 'Todo 1',
__typename: 'Todo',
},
],
},
query: gql`
query {
todos {
id
name
description
}
`,
});
}).toThrowError(/Missing field 'description' /);
}
`,
});
});
});

Expand Down Expand Up @@ -1119,7 +1117,7 @@ describe('ApolloClient', () => {
});
});

it('should warn when the data provided does not match the fragment shape', () => {
withErrorSpy(it, 'should warn when the data provided does not match the fragment shape', () => {
const client = new ApolloClient({
link: ApolloLink.empty(),
cache: new InMemoryCache({
Expand All @@ -1128,18 +1126,16 @@ describe('ApolloClient', () => {
}),
});

expect(() => {
client.writeFragment({
data: { __typename: 'Bar', i: 10 },
id: 'bar',
fragment: gql`
fragment fragmentBar on Bar {
i
e
}
`,
});
}).toThrowError(/Missing field 'e' /);
client.writeFragment({
data: { __typename: 'Bar', i: 10 },
id: 'bar',
fragment: gql`
fragment fragmentBar on Bar {
i
e
}
`,
});
});

describe('change will call observable next', () => {
Expand Down
39 changes: 39 additions & 0 deletions src/__tests__/__snapshots__/ApolloClient.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,25 @@ Object {
}
`;

exports[`ApolloClient writeFragment should warn when the data provided does not match the fragment shape 1`] = `
[MockFunction] {
"calls": Array [
Array [
"Missing field 'e' while writing result {
\\"__typename\\": \\"Bar\\",
\\"i\\": 10
}",
],
],
"results": Array [
Object {
"type": "return",
"value": undefined,
},
],
}
`;

exports[`ApolloClient writeFragment will write some deeply nested data into the store at any id 1`] = `
Object {
"__META": Object {
Expand Down Expand Up @@ -359,6 +378,26 @@ Object {
}
`;

exports[`ApolloClient writeQuery should warn when the data provided does not match the query shape 1`] = `
[MockFunction] {
"calls": Array [
Array [
"Missing field 'description' while writing result {
\\"id\\": \\"1\\",
\\"name\\": \\"Todo 1\\",
\\"__typename\\": \\"Todo\\"
}",
],
],
"results": Array [
Object {
"type": "return",
"value": undefined,
},
],
}
`;

exports[`ApolloClient writeQuery will write some deeply nested data to the store 1`] = `
Object {
"ROOT_QUERY": Object {
Expand Down
21 changes: 21 additions & 0 deletions src/__tests__/__snapshots__/client.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,24 @@ Object {
},
}
`;

exports[`client should warn if server returns wrong data 1`] = `
[MockFunction] {
"calls": Array [
Array [
"Missing field 'description' while writing result {
\\"id\\": \\"1\\",
\\"name\\": \\"Todo 1\\",
\\"price\\": 100,
\\"__typename\\": \\"Todo\\"
}",
],
],
"results": Array [
Object {
"type": "return",
"value": undefined,
},
],
}
`;
1 change: 1 addition & 0 deletions src/__tests__/__snapshots__/exports.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ Array [
"mockSingleLink",
"stripSymbols",
"subscribeAndCount",
"withErrorSpy",
]
`;

Expand Down
20 changes: 20 additions & 0 deletions src/__tests__/__snapshots__/mutationResults.ts.snap
Original file line number Diff line number Diff line change
@@ -1,5 +1,25 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`mutation results should warn when the result fields don't match the query fields 1`] = `
[MockFunction] {
"calls": Array [
Array [
"Missing field 'description' while writing result {
\\"id\\": \\"2\\",
\\"name\\": \\"Todo 2\\",
\\"__typename\\": \\"createTodo\\"
}",
],
],
"results": Array [
Object {
"type": "return",
"value": undefined,
},
],
}
`;

exports[`mutation results should write results to cache according to errorPolicy 1`] = `Object {}`;

exports[`mutation results should write results to cache according to errorPolicy 2`] = `
Expand Down
35 changes: 21 additions & 14 deletions src/__tests__/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
stripSymbols,
subscribeAndCount,
mockSingleLink,
withErrorSpy,
} from '../testing';

describe('client', () => {
Expand Down Expand Up @@ -2081,6 +2082,7 @@ describe('client', () => {
resolve();
});
});

itAsync('should allow errors to be returned from a mutation', (resolve, reject) => {
const mutation = gql`
mutation {
Expand All @@ -2102,7 +2104,12 @@ describe('client', () => {
const client = new ApolloClient({
link: mockSingleLink({
request: { query: mutation },
result: { data, errors },
result: {
errors,
data: {
newPerson: data,
},
},
}).setOnError(reject),
cache: new InMemoryCache({ addTypename: false }),
});
Expand All @@ -2112,7 +2119,9 @@ describe('client', () => {
expect(result.errors).toBeDefined();
expect(result.errors!.length).toBe(1);
expect(result.errors![0].message).toBe(errors[0].message);
expect(result.data).toEqual(data);
expect(result.data).toEqual({
newPerson: data,
});
resolve();
})
.catch((error: ApolloError) => {
Expand All @@ -2132,9 +2141,11 @@ describe('client', () => {
}
`;
const data = {
person: {
firstName: 'John',
lastName: 'Smith',
newPerson: {
person: {
firstName: 'John',
lastName: 'Smith',
},
},
};
const errors = [new Error('Some kind of GraphQL error.')];
Expand Down Expand Up @@ -2631,7 +2642,7 @@ describe('client', () => {
}).then(resolve, reject);
});

itAsync('should warn if server returns wrong data', (resolve, reject) => {
withErrorSpy(itAsync, 'should warn if server returns wrong data', (resolve, reject) => {
const query = gql`
query {
todos {
Expand All @@ -2654,6 +2665,7 @@ describe('client', () => {
],
},
};

const link = mockSingleLink({
request: { query },
result,
Expand All @@ -2666,14 +2678,9 @@ describe('client', () => {
}),
});

return client.query({ query }).then(
result => {
fail("should have errored");
},
error => {
expect(error.message).toMatch(/Missing field 'description' /);
},
).then(resolve, reject);
return client.query({ query }).then(({ data }) => {
expect(data).toEqual(result.data);
}).then(resolve, reject);
});

itAsync('runs a query with the connection directive and writes it to the store key defined in the directive', (resolve, reject) => {
Expand Down
Loading