Skip to content

Commit

Permalink
(graphcache) - Handle fields with associated GraphQLError as cache mi…
Browse files Browse the repository at this point in the history
…sses and provide errors to updaters (#1356)

* Add Context.path tracking the currently field alias path

* Remove active ctx.path tracking from query in production

It's not needed to track current errors from results there,
so it can alternatively be fully removed.

* Add note to ResolveInfo type on why path isn't exposed via types

* Provide the current field's GraphQLError on an updater's context

* Update docs to add information about the error field

* Overwrite null values as undefined when field has an associated error

When a field has an associated GraphQLError then the `fieldValue` should
be written as `undefined` rather than `null`.

* Replace isFieldMissing with getFieldError

* Isolate path/errorMap on context in context.__internal

This is to further hide the implementation from the user and
to ensure that sub-write methods like `updateQuery` or `writeFragment`
on the Store don't interfere with this logic as they'd build up their
own paths.

* Fix leftover global errorMap in shared.ts

* Add tests for errored fields set to undefined rather than null

* Add changeset

* Move updateContext in write.ts' updater call

* Deduplicate writeOptimistic logic with startWrite
  • Loading branch information
kitten authored Feb 4, 2021
1 parent 77fe772 commit 6a68709
Show file tree
Hide file tree
Showing 8 changed files with 261 additions and 100 deletions.
5 changes: 5 additions & 0 deletions .changeset/smart-emus-jam.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@urql/exchange-graphcache': major
---

Add improved error awareness to Graphcache. When Graphcache now receives a `GraphQLError` (via a `CombinedError`) it checks whether the `GraphQLError`'s `path` matches up with `null` values in the `data`. Any `null` values that the write operation now sees in the data will be replaced with a "cache miss" value (i.e. `undefined`) when it has an associated error. This means that errored fields from your GraphQL API will be marked as uncached and won't be cached. Instead the client will now attempt a refetch of the data so that errors aren't preventing future refetches or with schema awareness it will attempt a refetch automatically. Additionally, the `updates` functions will now be able to check whether the current field has any errors associated with it with `info.error`.
30 changes: 19 additions & 11 deletions docs/api/graphcache.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,13 @@ An `UpdateResolver` receives four arguments when it's called: `result`, `args`,
| `cache` | `Cache` | The cache using which data can be read or written. [See `Cache`.](#cache) |
| `info` | `Info` | Additional metadata and information about the current operation and the current field. [See `Info`.](#info) |

It's possible to derive more information about the current update using the `info` argument. For
instance this metadata contains the current `fieldName` of the updater which may be used to make an
updater function more reusable, along with `parentKey` and other key fields. It also contains
`variables` and `fragments` which remain the same for the entire write operation, and additionally
it may have the `error` field set to describe whether the current field is `null` because the API
encountered a `GraphQLError`.

[Read more about how to set up `updates` on the "Custom Updates"
page.](../graphcache/custom-updates.md)

Expand Down Expand Up @@ -463,17 +470,18 @@ This is a metadata object that is passed to every resolver and updater function.
information about the current GraphQL document and query, and also some information on the current
field that a given resolver or updater is called on.
| Argument | Type | Description |
| ---------------- | -------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| `parent` | `Data` | The field's parent entity's data, as it was written or read up until now, which means it may be incomplete. [Use `cache.resolve`](#resolve) to read from it. |
| `parentTypeName` | `string` | The field's parent entity's typename |
| `parentKey` | `string` | The field's parent entity's cache key (if any) |
| `parentFieldKey` | `string` | The current key's cache key, which is the parent entity's key combined with the current field's key (This is mostly obsolete) |
| `fieldName` | `string` | The current field's name |
| `fragments` | `{ [name: string]: FragmentDefinitionNode }` | A dictionary of fragments from the current GraphQL document |
| `variables` | `object` | The current GraphQL operation's variables (may be an empty object) |
| `partial` | `?boolean` | This may be set to `true` at any point in time (by your custom resolver or by _Graphcache_) to indicate that some data is uncached and missing |
| `optimistic` | `?boolean` | This is only `true` when an optimistic mutation update is running |
| Argument | Type | Description |
| ---------------- | -------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `parent` | `Data` | The field's parent entity's data, as it was written or read up until now, which means it may be incomplete. [Use `cache.resolve`](#resolve) to read from it. |
| `parentTypeName` | `string` | The field's parent entity's typename |
| `parentKey` | `string` | The field's parent entity's cache key (if any) |
| `parentFieldKey` | `string` | The current key's cache key, which is the parent entity's key combined with the current field's key (This is mostly obsolete) |
| `fieldName` | `string` | The current field's name |
| `fragments` | `{ [name: string]: FragmentDefinitionNode }` | A dictionary of fragments from the current GraphQL document |
| `variables` | `object` | The current GraphQL operation's variables (may be an empty object) |
| `error` | `GraphQLError \| undefined` | The current GraphQLError for a given field. This will always be `undefined` for resolvers and optimistic updaters, but may be present for updaters when the API has returned an error for a given field. |
| `partial` | `?boolean` | This may be set to `true` at any point in time (by your custom resolver or by _Graphcache_) to indicate that some data is uncached and missing |
| `optimistic` | `?boolean` | This is only `true` when an optimistic mutation update is running |
> **Note:** Using `info` is regarded as a last resort. Please only use information from it if
> there's no other solution to get to the metadata you need. We don't regard the `Info` API as
Expand Down
9 changes: 7 additions & 2 deletions exchanges/graphcache/src/cacheExchange.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,13 @@ export const cacheExchange = (opts?: CacheExchangeOpts): Exchange => ({
if (result.data) {
// Write the result to cache and collect all dependencies that need to be
// updated
const writeDependencies = write(store, operation, result.data, key)
.dependencies;
const writeDependencies = write(
store,
operation,
result.data,
result.error,
key
).dependencies;
collectPendingOperations(pendingOperations, writeDependencies);

const queryResult = query(store, operation, result.data, key);
Expand Down
43 changes: 37 additions & 6 deletions exchanges/graphcache/src/operations/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,18 @@ const readRoot = (
while ((node = iterate())) {
const fieldAlias = getFieldAlias(node);
const fieldValue = originalData[fieldAlias];
// Add the current alias to the walked path before processing the field's value
if (process.env.NODE_ENV !== 'production')
ctx.__internal.path.push(fieldAlias);
// Process the root field's value
if (node.selectionSet && fieldValue !== null) {
const fieldData = ensureData(fieldValue);
data[fieldAlias] = readRootField(ctx, getSelectionSet(node), fieldData);
} else {
data[fieldAlias] = fieldValue;
}
// After processing the field, remove the current alias from the path again
if (process.env.NODE_ENV !== 'production') ctx.__internal.path.pop();
}

return data;
Expand All @@ -147,8 +153,15 @@ const readRootField = (
): Data | NullArray<Data> | null => {
if (Array.isArray(originalData)) {
const newData = new Array(originalData.length);
for (let i = 0, l = originalData.length; i < l; i++)
for (let i = 0, l = originalData.length; i < l; i++) {
// Add the current index to the walked path before reading the field's value
if (process.env.NODE_ENV !== 'production') ctx.__internal.path.push(i);
// Recursively read the root field's value
newData[i] = readRootField(ctx, select, originalData[i]);
// After processing the field, remove the current index from the path
if (process.env.NODE_ENV !== 'production') ctx.__internal.path.pop();
}

return newData;
} else if (originalData === null) {
return null;
Expand Down Expand Up @@ -289,14 +302,20 @@ const readSelection = (
isFieldAvailableOnType(store.schema, typename, fieldName);
}

// We directly assign typenames and skip the field afterwards
if (fieldName === '__typename') {
data[fieldAlias] = typename;
continue;
}

// We temporarily store the data field in here, but undefined
// means that the value is missing from the cache
let dataFieldValue: void | DataField;
// Add the current alias to the walked path before processing the field's value
if (process.env.NODE_ENV !== 'production')
ctx.__internal.path.push(fieldAlias);

if (fieldName === '__typename') {
data[fieldAlias] = typename;
continue;
} else if (resultValue !== undefined && node.selectionSet === undefined) {
if (resultValue !== undefined && node.selectionSet === undefined) {
// The field is a scalar and can be retrieved directly from the result
dataFieldValue = resultValue;
} else if (
Expand Down Expand Up @@ -377,6 +396,8 @@ const readSelection = (
}
}

// After processing the field, remove the current alias from the path again
if (process.env.NODE_ENV !== 'production') ctx.__internal.path.pop();
// Now that dataFieldValue has been retrieved it'll be set on data
// If it's uncached (undefined) but nullable we can continue assembling
// a partial query result
Expand Down Expand Up @@ -420,6 +441,8 @@ const resolveResolverResult = (
!store.schema || isListNullable(store.schema, typename, fieldName);
const data = new Array(result.length);
for (let i = 0, l = result.length; i < l; i++) {
// Add the current index to the walked path before reading the field's value
if (process.env.NODE_ENV !== 'production') ctx.__internal.path.push(i);
// Recursively read resolver result
const childResult = resolveResolverResult(
ctx,
Expand All @@ -431,7 +454,9 @@ const resolveResolverResult = (
prevData != null ? prevData[i] : undefined,
result[i]
);

// After processing the field, remove the current index from the path
if (process.env.NODE_ENV !== 'production') ctx.__internal.path.pop();
// Check the result for cache-missed values
if (childResult === undefined && !_isListNullable) {
return undefined;
} else {
Expand Down Expand Up @@ -478,6 +503,9 @@ const resolveLink = (
store.schema && isListNullable(store.schema, typename, fieldName);
const newLink = new Array(link.length);
for (let i = 0, l = link.length; i < l; i++) {
// Add the current index to the walked path before reading the field's value
if (process.env.NODE_ENV !== 'production') ctx.__internal.path.push(i);
// Recursively read the link
const childLink = resolveLink(
ctx,
link[i],
Expand All @@ -486,6 +514,9 @@ const resolveLink = (
select,
prevData != null ? prevData[i] : undefined
);
// After processing the field, remove the current index from the path
if (process.env.NODE_ENV !== 'production') ctx.__internal.path.pop();
// Check the result for cache-missed values
if (childLink === undefined && !_isListNullable) {
return undefined;
} else {
Expand Down
67 changes: 53 additions & 14 deletions exchanges/graphcache/src/operations/shared.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import { FieldNode, InlineFragmentNode, FragmentDefinitionNode } from 'graphql';
import { CombinedError } from '@urql/core';
import {
GraphQLError,
FieldNode,
InlineFragmentNode,
FragmentDefinitionNode,
} from 'graphql';

import {
isInlineFragment,
Expand All @@ -24,31 +30,63 @@ export interface Context {
parentFieldKey: string;
parent: Data;
fieldName: string;
error: GraphQLError | undefined;
partial: boolean;
optimistic: boolean;
__internal: {
path: Array<string | number>;
errorMap: { [path: string]: GraphQLError } | undefined;
};
}

export const contextRef: { current: Context | null } = { current: null };

// Checks whether the current data field is a cache miss because of a GraphQLError
export const getFieldError = (ctx: Context): GraphQLError | undefined =>
ctx.__internal.path.length > 0 && ctx.__internal.errorMap
? ctx.__internal.errorMap[ctx.__internal.path.join('.')]
: undefined;

export const makeContext = (
store: Store,
variables: Variables,
fragments: Fragments,
typename: string,
entityKey: string,
optimistic?: boolean
): Context => ({
store,
variables,
fragments,
parent: { __typename: typename },
parentTypeName: typename,
parentKey: entityKey,
parentFieldKey: '',
fieldName: '',
partial: false,
optimistic: !!optimistic,
});
optimistic?: boolean,
error?: CombinedError | undefined
): Context => {
const ctx: Context = {
store,
variables,
fragments,
parent: { __typename: typename },
parentTypeName: typename,
parentKey: entityKey,
parentFieldKey: '',
fieldName: '',
error: undefined,
partial: false,
optimistic: !!optimistic,
__internal: {
path: [],
errorMap: undefined,
},
};

if (error && error.graphQLErrors) {
for (let i = 0; i < error.graphQLErrors.length; i++) {
const graphQLError = error.graphQLErrors[i];
if (graphQLError.path && graphQLError.path.length) {
if (!ctx.__internal.errorMap)
ctx.__internal.errorMap = Object.create(null);
ctx.__internal.errorMap![graphQLError.path.join('.')] = graphQLError;
}
}
}

return ctx;
};

export const updateContext = (
ctx: Context,
Expand All @@ -64,6 +102,7 @@ export const updateContext = (
ctx.parentKey = entityKey;
ctx.parentFieldKey = fieldKey;
ctx.fieldName = fieldName;
ctx.error = getFieldError(ctx);
};

const isFragmentHeuristicallyMatching = (
Expand Down
75 changes: 74 additions & 1 deletion exchanges/graphcache/src/operations/write.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint-disable @typescript-eslint/no-var-requires */

import { gql } from '@urql/core';
import { gql, CombinedError } from '@urql/core';
import { minifyIntrospectionQuery } from '@urql/introspection';

import { write } from './write';
Expand Down Expand Up @@ -174,4 +174,77 @@ describe('Query', () => {
// The field must still be `'test'`
expect(InMemoryData.readRecord('Query', 'field')).toBe('test');
});

it('should write errored records as undefined rather than null', () => {
const query = gql`
{
missingField
setField
}
`;

write(
store,
{ query },
{ missingField: null, setField: 'test' } as any,
new CombinedError({
graphQLErrors: [
{
message: 'Test',
path: ['missingField'],
},
],
})
);

InMemoryData.initDataState('read', store.data, null);

// The setField must still be `'test'`
expect(InMemoryData.readRecord('Query', 'setField')).toBe('test');
// The missingField must still be `undefined`
expect(InMemoryData.readRecord('Query', 'missingField')).toBe(undefined);
});

it('should write errored links as undefined rather than null', () => {
const query = gql`
{
missingTodoItem: todos {
id
text
}
missingTodo: todo {
id
text
}
}
`;

write(
store,
{ query },
{
missingTodoItem: [null, { __typename: 'Todo', id: 1, text: 'Learn' }],
missingTodo: null,
} as any,
new CombinedError({
graphQLErrors: [
{
message: 'Test',
path: ['missingTodoItem', 0],
},
{
message: 'Test',
path: ['missingTodo'],
},
],
})
);

InMemoryData.initDataState('read', store.data, null);
expect(InMemoryData.readLink('Query', 'todos')).toEqual([
undefined,
'Todo:1',
]);
expect(InMemoryData.readLink('Query', 'todo')).toEqual(undefined);
});
});
Loading

0 comments on commit 6a68709

Please sign in to comment.