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

Throw when writing inconsistent query/data to InMemoryCache. #6055

Merged
merged 3 commits into from
Mar 17, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
9 changes: 4 additions & 5 deletions src/__tests__/ApolloClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { ApolloLink } from '../link/core/ApolloLink';
import { HttpLink } from '../link/http/HttpLink';
import { InMemoryCache } from '../cache/inmemory/inMemoryCache';
import { stripSymbols } from '../utilities/testing/stripSymbols';
import { withWarning } from '../utilities/testing/wrap';
import { ApolloClient } from '../';
import { DefaultOptions } from '../ApolloClient';
import { FetchPolicy, QueryOptions } from '../core/watchQueryOptions';
Expand Down Expand Up @@ -827,7 +826,7 @@ describe('ApolloClient', () => {
}),
});

return withWarning(() => {
expect(() => {
client.writeQuery({
data: {
todos: [
Expand All @@ -848,7 +847,7 @@ describe('ApolloClient', () => {
}
`,
});
}, /Missing field description/);
}).toThrowError(/Missing field description/);
});
});

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

return withWarning(() => {
expect(() => {
client.writeFragment({
data: { __typename: 'Bar', i: 10 },
id: 'bar',
Expand All @@ -1120,7 +1119,7 @@ describe('ApolloClient', () => {
}
`,
});
}, /Missing field e/);
}).toThrowError(/Missing field e/);
});

describe('change will call observable next', () => {
Expand Down
46 changes: 7 additions & 39 deletions src/__tests__/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { WatchQueryOptions, FetchPolicy, WatchQueryFetchPolicy } from '../core/w
import { ApolloError } from '../errors/ApolloError';
import { ApolloClient } from '..';
import subscribeAndCount from '../utilities/testing/subscribeAndCount';
import { withWarning } from '../utilities/testing/wrap';
import { itAsync } from '../utilities/testing/itAsync';
import { mockSingleLink } from '../utilities/testing/mocking/mockLink';
import { NetworkStatus, ObservableQuery } from '../core';
Expand Down Expand Up @@ -625,41 +624,6 @@ describe('client', () => {
});
});

itAsync.skip('should pass a network error correctly on a query using an observable network interface with a warning', (resolve, reject) => {
withWarning(() => {
const query = gql`
query people {
allPeople(first: 1) {
people {
name
}
}
}
`;

const networkError = new Error('Some kind of network error.');

const link = ApolloLink.from([
() => {
return new Observable(_ => {
throw networkError;
});
},
]);

const client = new ApolloClient({
link,
cache: new InMemoryCache({ addTypename: false }),
});

client.query({ query }).catch((error: ApolloError) => {
expect(error.networkError).toBeDefined();
expect(error.networkError!.message).toEqual(networkError.message);
resolve();
});
}, /deprecated/);
});

itAsync('should pass a network error correctly on a query with apollo-link network interface', (resolve, reject) => {
const query = gql`
query people {
Expand Down Expand Up @@ -2620,9 +2584,13 @@ describe('client', () => {
}),
});

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

Expand Down
78 changes: 41 additions & 37 deletions src/__tests__/mutationResults.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { ApolloLink } from '../link/core/ApolloLink';
import { mockSingleLink } from '../utilities/testing/mocking/mockLink';
import { ApolloClient } from '..';
import { InMemoryCache } from '../cache/inmemory/inMemoryCache';
import { withWarning } from '../utilities/testing/wrap';
import { itAsync } from '../utilities/testing/itAsync';

describe('mutation results', () => {
Expand Down Expand Up @@ -358,45 +357,50 @@ describe('mutation results', () => {
},
};

return withWarning(() => {
const { client, obsQuery } = setupObsQuery(
reject,
{
request: { query: queryTodos },
result: queryTodosResult,
},
{
request: { query: mutationTodo },
result: mutationTodoResult,
},
);
const { client, obsQuery } = setupObsQuery(
reject,
{
request: { query: queryTodos },
result: queryTodosResult,
},
{
request: { query: mutationTodo },
result: mutationTodoResult,
},
);

return obsQuery.result().then(() => {
// we have to actually subscribe to the query to be able to update it
return new Promise(resolve => {
handle = client.watchQuery({ query: queryTodos });
subscriptionHandle = handle.subscribe({
next(res: any) {
counter++;
resolve(res);
},
});
});
}).then(() => client.mutate({
mutation: mutationTodo,
updateQueries: {
todos: (prev, { mutationResult }) => {
const newTodo = (mutationResult as any).data.createTodo;
const newResults = {
todos: [...(prev as any).todos, newTodo],
};
return newResults;
return obsQuery.result().then(() => {
// we have to actually subscribe to the query to be able to update it
return new Promise(resolve => {
handle = client.watchQuery({ query: queryTodos });
subscriptionHandle = handle.subscribe({
next(res: any) {
counter++;
resolve(res);
},
});
});
}).then(() => client.mutate({
mutation: mutationTodo,
updateQueries: {
todos: (prev, { mutationResult }) => {
const newTodo = (mutationResult as any).data.createTodo;
const newResults = {
todos: [...(prev as any).todos, newTodo],
};
return newResults;
},
})).then(
() => subscriptionHandle.unsubscribe()
).then(resolve, reject);
}, /Missing field description/);
},
})).then(
() => {
subscriptionHandle.unsubscribe();
fail("should have errored");
},
error => {
subscriptionHandle.unsubscribe();
expect(error.message).toMatch(/Missing field description /);
},
).then(resolve, reject);
});

describe('updateQueries', () => {
Expand Down
9 changes: 4 additions & 5 deletions src/cache/inmemory/__tests__/roundtrip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { DocumentNode } from 'graphql';
import gql from 'graphql-tag';

import { withError } from './diffAgainstStore';
import { withWarning } from './writeToStore';
import { EntityStore } from '../entityStore';
import { StoreReader } from '../readFromStore';
import { StoreWriter } from '../writeToStore';
Expand Down Expand Up @@ -311,7 +310,7 @@ describe('roundtrip', () => {
// XXX this test is weird because it assumes the server returned an incorrect result
// However, the user may have written this result with client.writeQuery.
it('should throw an error on two of the same inline fragment types', () => {
return withWarning(() => expect(() => {
expect(() => {
storeRoundtrip(
gql`
query {
Expand All @@ -337,7 +336,7 @@ describe('roundtrip', () => {
],
},
);
}).toThrowError(/Can\'t find field rank on object/));
}).toThrowError(/Missing field rank /);
});

it('should resolve fields it can on interface with non matching inline fragments', () => {
Expand Down Expand Up @@ -451,7 +450,7 @@ describe('roundtrip', () => {
});

it('should throw on error on two of the same spread fragment types', () => {
withWarning(() => expect(() => {
expect(() => {
storeRoundtrip(
gql`
fragment jediSide on Jedi {
Expand Down Expand Up @@ -481,7 +480,7 @@ describe('roundtrip', () => {
],
},
);
}).toThrowError(/Can\'t find field rank on object/));
}).toThrowError(/Missing field rank /);
});

it('should resolve on @include and @skip with inline fragments', () => {
Expand Down
39 changes: 9 additions & 30 deletions src/cache/inmemory/__tests__/writeToStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,6 @@ import { defaultNormalizedCacheFactory } from '../entityStore';
import { InMemoryCache } from '../inMemoryCache';
import { Policies } from '../policies';

export function withWarning(func: Function, regex?: RegExp) {
let message: string = null as never;
const oldWarn = console.warn;

console.warn = (m: string) => (message = m);

return Promise.resolve(func()).then(val => {
if (regex) {
expect(message).toMatch(regex);
}
console.warn = oldWarn;
return val;
});
}

const getIdField = ({ id }: { id: string }) => id;

describe('writing to the store', () => {
Expand Down Expand Up @@ -1563,14 +1548,12 @@ describe('writing to the store', () => {
}),
});

return withWarning(() => {
const newStore = writer.writeQueryToStore({
expect(() => {
writer.writeQueryToStore({
query,
result,
});

expect((newStore as any).lookup('1')).toEqual(result.todos[0]);
}, /Missing field description/);
}).toThrowError(/Missing field description/);
});

it('should warn when it receives the wrong data inside a fragment', () => {
Expand Down Expand Up @@ -1617,14 +1600,12 @@ describe('writing to the store', () => {
}),
});

return withWarning(() => {
const newStore = writer.writeQueryToStore({
expect(() => {
writer.writeQueryToStore({
query: queryWithInterface,
result,
});

expect((newStore as any).lookup('1')).toEqual(result.todos[0]);
}, /Missing field price/);
}).toThrowError(/Missing field price/);
});

it('should warn if a result is missing __typename when required', () => {
Expand All @@ -1645,14 +1626,12 @@ describe('writing to the store', () => {
}),
});

return withWarning(() => {
const newStore = writer.writeQueryToStore({
expect(() => {
writer.writeQueryToStore({
query: addTypenameToDocument(query),
result,
});

expect((newStore as any).lookup('1')).toEqual(result.todos[0]);
}, /Missing field __typename/);
}).toThrowError(/Missing field __typename/);
});

it('should not warn if a field is null', () => {
Expand Down
17 changes: 4 additions & 13 deletions src/cache/inmemory/writeToStore.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { SelectionSetNode, FieldNode, DocumentNode } from 'graphql';
import { invariant } from 'ts-invariant';
import { InvariantError } from 'ts-invariant';

import {
createFragmentMap,
Expand All @@ -22,7 +22,7 @@ import {
StoreObject,
} from '../../utilities/graphql/storeUtils';

import { shouldInclude } from '../../utilities/graphql/directives';
import { shouldInclude, hasDirectives } from '../../utilities/graphql/directives';
import { cloneDeep } from '../../utilities/common/cloneDeep';

import { Policies, ReadMergeContext } from './policies';
Expand Down Expand Up @@ -229,18 +229,9 @@ export class StoreWriter {

} else if (
policies.usingPossibleTypes &&
!(
selection.directives &&
selection.directives.some(
({ name }) =>
name && (name.value === 'defer' || name.value === 'client'),
)
)
!hasDirectives(["defer", "client"], selection)
) {
// XXX We'd like to throw an error, but for backwards compatibility's sake
// we just print a warning for the time being.
//throw new WriteError(`Missing field ${resultFieldKey} in ${JSON.stringify(result, null, 2).substring(0, 100)}`);
invariant.warn(
Comment on lines -240 to -243
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the old // XXX comment that this PR addresses.

throw new InvariantError(
`Missing field ${resultFieldKey} in ${JSON.stringify(
result,
null,
Expand Down
23 changes: 15 additions & 8 deletions src/core/QueryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,14 +239,21 @@ export class QueryManager<TStore> {
self.mutationStore.markMutationResult(mutationId);

if (fetchPolicy !== 'no-cache') {
markMutationResult({
mutationId,
result,
document: mutation,
variables,
queryUpdatersById: generateUpdateQueriesInfo(),
update: updateWithProxyFn,
}, self.cache);
try {
markMutationResult({
mutationId,
result,
document: mutation,
variables,
queryUpdatersById: generateUpdateQueriesInfo(),
update: updateWithProxyFn,
}, self.cache);
} catch (e) {
error = new ApolloError({
networkError: e,
});
return;
}
}

storeResult = result as FetchResult<T>;
Expand Down
Loading