From fca7a9ca7ebdf3c307513eb853dc1444881a11c5 Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Tue, 13 Jun 2023 16:49:35 -0700 Subject: [PATCH] Catch errors thrown during .read() Reviewed By: josephsavona Differential Revision: D46500160 fbshipit-source-id: 958d87878faa941bc790500362a4e52f5ca651a0 --- .../__tests__/LiveResolvers-test.js | 33 +++++- .../__tests__/resolvers/LiveResolvers-test.js | 103 ++++++++++++++++++ .../QueryLiveResolverThrowsOnRead.js | 46 ++++++++ ...versTestHandlesErrorOnReadQuery.graphql.js | 99 +++++++++++++++++ ...rsTestHandlesErrorOnUpdateQuery.graphql.js | 99 +++++++++++++++++ .../LiveResolverCache.js | 58 +++++++--- 6 files changed, 417 insertions(+), 21 deletions(-) create mode 100644 packages/relay-runtime/store/__tests__/resolvers/QueryLiveResolverThrowsOnRead.js create mode 100644 packages/relay-runtime/store/__tests__/resolvers/__generated__/LiveResolversTestHandlesErrorOnReadQuery.graphql.js create mode 100644 packages/relay-runtime/store/__tests__/resolvers/__generated__/LiveResolversTestHandlesErrorOnUpdateQuery.graphql.js diff --git a/packages/react-relay/__tests__/LiveResolvers-test.js b/packages/react-relay/__tests__/LiveResolvers-test.js index 4184bce3e4b4d..abcad9c59ea8d 100644 --- a/packages/react-relay/__tests__/LiveResolvers-test.js +++ b/packages/react-relay/__tests__/LiveResolvers-test.js @@ -980,12 +980,33 @@ describe.each([ requiredFieldLogger.mockReset(); // Render with complete data - expect(() => { - TestRenderer.create(); - }).toThrow( - 'The resolver should throw earlier. It should have missing data.', + let renderer; + TestRenderer.act(() => { + renderer = TestRenderer.create( + , + ); + }); + + if (renderer == null) { + throw new Error('Renderer is expected to be defined.'); + } + + expect(requiredFieldLogger.mock.calls).toEqual([ + [ + { + error: new Error( + 'The resolver should throw earlier. It should have missing data.', + ), + fieldPath: 'node.resolver_that_throws', + kind: 'relay_resolver.error', + owner: 'LiveResolversTest8Query', + }, + ], + ]); + + expect(renderer.toJSON()).toEqual( + 'Bob: Unknown resolver_that_throws value', ); - expect(requiredFieldLogger.mock.calls).toEqual([]); }); test('apply optimistic updates to live resolver field', () => { @@ -1462,7 +1483,7 @@ describe.each([ expect(() => { GLOBAL_STORE.dispatch({type: 'INCREMENT'}); }).toThrowError( - 'Unexpected LiveState value returned from Relay Resolver internal field `RELAY_RESOLVER_LIVE_STATE_VALUE`. It is likely a bug in Relay, or a corrupt state of the relay store state Field Path `counter_suspends_when_odd`. Record `{"__id":"client:1:counter_suspends_when_odd","__typename":"__RELAY_RESOLVER__","__resolverValue":{"__LIVE_RESOLVER_SUSPENSE_SENTINEL":true},"__resolverLiveStateDirty":true,"__resolverError":null}`', + 'Unexpected LiveState value returned from Relay Resolver internal field `RELAY_RESOLVER_LIVE_STATE_VALUE`. It is likely a bug in Relay, or a corrupt state of the relay store state Field Path `counter_suspends_when_odd`. Record `{"__id":"client:1:counter_suspends_when_odd","__typename":"__RELAY_RESOLVER__","__resolverError":null,"__resolverValue":{"__LIVE_RESOLVER_SUSPENSE_SENTINEL":true},"__resolverLiveStateDirty":true}`.', ); expect(renderer.toJSON()).toEqual('Loading...'); }); diff --git a/packages/relay-runtime/store/__tests__/resolvers/LiveResolvers-test.js b/packages/relay-runtime/store/__tests__/resolvers/LiveResolvers-test.js index 89ee0c671c15e..35ba891ed9f18 100644 --- a/packages/relay-runtime/store/__tests__/resolvers/LiveResolvers-test.js +++ b/packages/relay-runtime/store/__tests__/resolvers/LiveResolvers-test.js @@ -240,3 +240,106 @@ test('Updates can be batched', () => { 'liveresolver.batch.end', ]); }); + +test('Errors thrown during _initial_ read() are caught as resolver errors', () => { + GLOBAL_STORE.dispatch({type: 'INCREMENT'}); + const source = RelayRecordSource.create({ + 'client:root': { + __id: 'client:root', + __typename: '__Root', + }, + }); + const operation = createOperationDescriptor( + graphql` + query LiveResolversTestHandlesErrorOnReadQuery { + counter_throws_when_odd + } + `, + {}, + ); + const store = new LiveResolverStore(source, { + gcReleaseBufferSize: 0, + }); + const environment = new RelayModernEnvironment({ + network: RelayNetwork.create(jest.fn()), + store, + }); + + const snapshot = environment.lookup(operation.fragment); + expect(snapshot.relayResolverErrors).toEqual([ + { + error: Error('What?'), + field: { + owner: 'LiveResolversTestHandlesErrorOnReadQuery', + path: 'counter_throws_when_odd', + }, + }, + ]); + const data: $FlowExpectedError = snapshot.data; + expect(data.counter_throws_when_odd).toBe(null); +}); + +test('Errors thrown during read() _after update_ are caught as resolver errors', () => { + const source = RelayRecordSource.create({ + 'client:root': { + __id: 'client:root', + __typename: '__Root', + }, + }); + const operation = createOperationDescriptor( + graphql` + query LiveResolversTestHandlesErrorOnUpdateQuery { + counter_throws_when_odd + } + `, + {}, + ); + const store = new LiveResolverStore(source, { + gcReleaseBufferSize: 0, + }); + const environment = new RelayModernEnvironment({ + network: RelayNetwork.create(jest.fn()), + store, + }); + + const snapshot = environment.lookup(operation.fragment); + + const handler = jest.fn<[Snapshot], void>(); + environment.subscribe(snapshot, handler); + + // Confirm there are no initial errors + expect(snapshot.relayResolverErrors).toEqual([]); + const data: $FlowExpectedError = snapshot.data; + expect(data.counter_throws_when_odd).toBe(0); + + // This should trigger a read that throws + GLOBAL_STORE.dispatch({type: 'INCREMENT'}); + + expect(handler).toHaveBeenCalled(); + + const nextSnapshot = handler.mock.calls[0][0]; + + expect(nextSnapshot.relayResolverErrors).toEqual([ + { + error: Error('What?'), + field: { + owner: 'LiveResolversTestHandlesErrorOnUpdateQuery', + path: 'counter_throws_when_odd', + }, + }, + ]); + const nextData: $FlowExpectedError = nextSnapshot.data; + expect(nextData.counter_throws_when_odd).toBe(null); + + handler.mockReset(); + + // Put the live resolver back into a state where it is valid + GLOBAL_STORE.dispatch({type: 'INCREMENT'}); + + const finalSnapshot = handler.mock.calls[0][0]; + + // Confirm there are no initial errors + expect(finalSnapshot.relayResolverErrors).toEqual([]); + const finalData: $FlowExpectedError = finalSnapshot.data; + expect(finalData.counter_throws_when_odd).toBe(2); +}); diff --git a/packages/relay-runtime/store/__tests__/resolvers/QueryLiveResolverThrowsOnRead.js b/packages/relay-runtime/store/__tests__/resolvers/QueryLiveResolverThrowsOnRead.js new file mode 100644 index 0000000000000..6f971c4bc82a9 --- /dev/null +++ b/packages/relay-runtime/store/__tests__/resolvers/QueryLiveResolverThrowsOnRead.js @@ -0,0 +1,46 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow strict-local + * @format + * @oncall relay + */ + +'use strict'; + +import type {LiveState} from '../../experimental-live-resolvers/LiveResolverStore'; + +const {GLOBAL_STORE, Selectors} = require('./ExampleExternalStateStore'); + +/** + * @RelayResolver Query.counter_throws_when_odd: Int + * @live + * + * A @live resolver that throws when counter is odd. Useful for testing + * behavior of live resolvers that throw on read. + */ + +function counter_throws_when_odd(): LiveState { + return { + read() { + const number = Selectors.getNumber(GLOBAL_STORE.getState()); + if (number % 2 !== 0) { + throw new Error('What?'); + } else { + return number; + } + }, + subscribe(cb): () => void { + // Here we could try to run the selector and short-circuit if + // the value has not changed, but for now we'll over-notify. + return GLOBAL_STORE.subscribe(cb); + }, + }; +} + +module.exports = { + counter_throws_when_odd, +}; diff --git a/packages/relay-runtime/store/__tests__/resolvers/__generated__/LiveResolversTestHandlesErrorOnReadQuery.graphql.js b/packages/relay-runtime/store/__tests__/resolvers/__generated__/LiveResolversTestHandlesErrorOnReadQuery.graphql.js new file mode 100644 index 0000000000000..23ae8698c923a --- /dev/null +++ b/packages/relay-runtime/store/__tests__/resolvers/__generated__/LiveResolversTestHandlesErrorOnReadQuery.graphql.js @@ -0,0 +1,99 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @oncall relay + * + * @generated SignedSource<> + * @flow + * @lightSyntaxTransform + * @nogrep + */ + +/* eslint-disable */ + +'use strict'; + +/*:: +import type { ClientRequest, ClientQuery } from 'relay-runtime'; +import type { LiveState } from "relay-runtime/store/experimental-live-resolvers/LiveResolverStore"; +import {counter_throws_when_odd as queryCounterThrowsWhenOddResolverType} from "../QueryLiveResolverThrowsOnRead.js"; +// Type assertion validating that `queryCounterThrowsWhenOddResolverType` resolver is correctly implemented. +// A type error here indicates that the type signature of the resolver module is incorrect. +(queryCounterThrowsWhenOddResolverType: () => LiveState); +export type LiveResolversTestHandlesErrorOnReadQuery$variables = {||}; +export type LiveResolversTestHandlesErrorOnReadQuery$data = {| + +counter_throws_when_odd: ?number, +|}; +export type LiveResolversTestHandlesErrorOnReadQuery = {| + response: LiveResolversTestHandlesErrorOnReadQuery$data, + variables: LiveResolversTestHandlesErrorOnReadQuery$variables, +|}; +*/ + +var node/*: ClientRequest*/ = { + "fragment": { + "argumentDefinitions": [], + "kind": "Fragment", + "metadata": null, + "name": "LiveResolversTestHandlesErrorOnReadQuery", + "selections": [ + { + "kind": "ClientExtension", + "selections": [ + { + "alias": null, + "args": null, + "fragment": null, + "kind": "RelayLiveResolver", + "name": "counter_throws_when_odd", + "resolverModule": require('./../QueryLiveResolverThrowsOnRead').counter_throws_when_odd, + "path": "counter_throws_when_odd" + } + ] + } + ], + "type": "Query", + "abstractKey": null + }, + "kind": "Request", + "operation": { + "argumentDefinitions": [], + "kind": "Operation", + "name": "LiveResolversTestHandlesErrorOnReadQuery", + "selections": [ + { + "kind": "ClientExtension", + "selections": [ + { + "name": "counter_throws_when_odd", + "args": null, + "fragment": null, + "kind": "RelayResolver", + "storageKey": null, + "isOutputType": true + } + ] + } + ] + }, + "params": { + "cacheID": "592ff0894b0f30b3727bc99191081c3e", + "id": null, + "metadata": {}, + "name": "LiveResolversTestHandlesErrorOnReadQuery", + "operationKind": "query", + "text": null + } +}; + +if (__DEV__) { + (node/*: any*/).hash = "47643627cf996a71e53ba0dfbbfdef54"; +} + +module.exports = ((node/*: any*/)/*: ClientQuery< + LiveResolversTestHandlesErrorOnReadQuery$variables, + LiveResolversTestHandlesErrorOnReadQuery$data, +>*/); diff --git a/packages/relay-runtime/store/__tests__/resolvers/__generated__/LiveResolversTestHandlesErrorOnUpdateQuery.graphql.js b/packages/relay-runtime/store/__tests__/resolvers/__generated__/LiveResolversTestHandlesErrorOnUpdateQuery.graphql.js new file mode 100644 index 0000000000000..33dd6e267826c --- /dev/null +++ b/packages/relay-runtime/store/__tests__/resolvers/__generated__/LiveResolversTestHandlesErrorOnUpdateQuery.graphql.js @@ -0,0 +1,99 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @oncall relay + * + * @generated SignedSource<<6216716280a5b6f13d885519bf6d068b>> + * @flow + * @lightSyntaxTransform + * @nogrep + */ + +/* eslint-disable */ + +'use strict'; + +/*:: +import type { ClientRequest, ClientQuery } from 'relay-runtime'; +import type { LiveState } from "relay-runtime/store/experimental-live-resolvers/LiveResolverStore"; +import {counter_throws_when_odd as queryCounterThrowsWhenOddResolverType} from "../QueryLiveResolverThrowsOnRead.js"; +// Type assertion validating that `queryCounterThrowsWhenOddResolverType` resolver is correctly implemented. +// A type error here indicates that the type signature of the resolver module is incorrect. +(queryCounterThrowsWhenOddResolverType: () => LiveState); +export type LiveResolversTestHandlesErrorOnUpdateQuery$variables = {||}; +export type LiveResolversTestHandlesErrorOnUpdateQuery$data = {| + +counter_throws_when_odd: ?number, +|}; +export type LiveResolversTestHandlesErrorOnUpdateQuery = {| + response: LiveResolversTestHandlesErrorOnUpdateQuery$data, + variables: LiveResolversTestHandlesErrorOnUpdateQuery$variables, +|}; +*/ + +var node/*: ClientRequest*/ = { + "fragment": { + "argumentDefinitions": [], + "kind": "Fragment", + "metadata": null, + "name": "LiveResolversTestHandlesErrorOnUpdateQuery", + "selections": [ + { + "kind": "ClientExtension", + "selections": [ + { + "alias": null, + "args": null, + "fragment": null, + "kind": "RelayLiveResolver", + "name": "counter_throws_when_odd", + "resolverModule": require('./../QueryLiveResolverThrowsOnRead').counter_throws_when_odd, + "path": "counter_throws_when_odd" + } + ] + } + ], + "type": "Query", + "abstractKey": null + }, + "kind": "Request", + "operation": { + "argumentDefinitions": [], + "kind": "Operation", + "name": "LiveResolversTestHandlesErrorOnUpdateQuery", + "selections": [ + { + "kind": "ClientExtension", + "selections": [ + { + "name": "counter_throws_when_odd", + "args": null, + "fragment": null, + "kind": "RelayResolver", + "storageKey": null, + "isOutputType": true + } + ] + } + ] + }, + "params": { + "cacheID": "da6bc143a73ab417777b9f52ab85616f", + "id": null, + "metadata": {}, + "name": "LiveResolversTestHandlesErrorOnUpdateQuery", + "operationKind": "query", + "text": null + } +}; + +if (__DEV__) { + (node/*: any*/).hash = "b0fe01bebd0ba17a2b27b256f6391a2d"; +} + +module.exports = ((node/*: any*/)/*: ClientQuery< + LiveResolversTestHandlesErrorOnUpdateQuery$variables, + LiveResolversTestHandlesErrorOnUpdateQuery$data, +>*/); diff --git a/packages/relay-runtime/store/experimental-live-resolvers/LiveResolverCache.js b/packages/relay-runtime/store/experimental-live-resolvers/LiveResolverCache.js index cee55589d5f4e..801eb7444f5bc 100644 --- a/packages/relay-runtime/store/experimental-live-resolvers/LiveResolverCache.js +++ b/packages/relay-runtime/store/experimental-live-resolvers/LiveResolverCache.js @@ -153,8 +153,19 @@ class LiveResolverCache implements ResolverCache { const evaluationResult = evaluate(); + RelayModernRecord.setValue( + linkedRecord, + RELAY_RESOLVER_SNAPSHOT_KEY, + evaluationResult.snapshot, + ); + RelayModernRecord.setValue( + linkedRecord, + RELAY_RESOLVER_ERROR_KEY, + evaluationResult.error, + ); + if (field.kind === RELAY_LIVE_RESOLVER) { - if (evaluationResult.resolverResult != undefined) { + if (evaluationResult.resolverResult != null) { if (__DEV__) { invariant( isLiveStateValue(evaluationResult.resolverResult), @@ -163,6 +174,10 @@ class LiveResolverCache implements ResolverCache { field.path, ); } + invariant( + evaluationResult.error == null, + 'Did not expect resolver to have both a value and an error.', + ); const liveState: LiveState = // $FlowFixMe[incompatible-type] - casting mixed evaluationResult.resolverResult; @@ -202,16 +217,7 @@ class LiveResolverCache implements ResolverCache { variables, ); } - RelayModernRecord.setValue( - linkedRecord, - RELAY_RESOLVER_SNAPSHOT_KEY, - evaluationResult.snapshot, - ); - RelayModernRecord.setValue( - linkedRecord, - RELAY_RESOLVER_ERROR_KEY, - evaluationResult.error, - ); + recordSource.set(linkedID, linkedRecord); // Link the resolver value record to the resolver field of the record being read: @@ -268,9 +274,9 @@ class LiveResolverCache implements ResolverCache { ); } - updatedDataIDs = this._setResolverValue( + updatedDataIDs = this._setLiveResolverValue( linkedRecord, - liveState.read(), + liveState, field, variables, ); @@ -361,9 +367,9 @@ class LiveResolverCache implements ResolverCache { ); // Store the current value, for this read, and future cached reads. - const updatedDataIDs = this._setResolverValue( + const updatedDataIDs = this._setLiveResolverValue( linkedRecord, - liveState.read(), + liveState, field, variables, ); @@ -466,6 +472,28 @@ class LiveResolverCache implements ResolverCache { } } + _setLiveResolverValue( + resolverRecord: Record, + liveValue: LiveState, + field: ReaderRelayResolver | ReaderRelayLiveResolver, + variables: Variables, + ): DataIDSet | null { + let value: null | mixed = null; + let resolverError: null | mixed = null; + try { + value = liveValue.read(); + } catch (e) { + resolverError = e; + } + + RelayModernRecord.setValue( + resolverRecord, + RELAY_RESOLVER_ERROR_KEY, + resolverError, + ); + return this._setResolverValue(resolverRecord, value, field, variables); + } + _setResolverValue( resolverRecord: Record, value: mixed,