From 94c8f0b6eff1f3ad996821e7543743b5fa1f27a9 Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Wed, 18 Sep 2024 21:18:54 -0700 Subject: [PATCH] Avoid running resolver code if root fragment throws with @required(action: THROW) --- packages/relay-runtime/store/RelayReader.js | 10 +- packages/relay-runtime/store/ResolverCache.js | 2 + .../relay-runtime/store/ResolverFragments.js | 20 +-- .../__tests__/RelayReader-Resolver-test.js | 47 ++++++ ...rResolverTestRequiredThrowQuery.graphql.js | 142 ++++++++++++++++++ .../UserRequiredThrowNameResolver.js | 50 ++++++ .../UserRequiredThrowNameResolver.graphql.js | 64 ++++++++ 7 files changed, 322 insertions(+), 13 deletions(-) create mode 100644 packages/relay-runtime/store/__tests__/__generated__/RelayReaderResolverTestRequiredThrowQuery.graphql.js create mode 100644 packages/relay-runtime/store/__tests__/resolvers/UserRequiredThrowNameResolver.js create mode 100644 packages/relay-runtime/store/__tests__/resolvers/__generated__/UserRequiredThrowNameResolver.graphql.js diff --git a/packages/relay-runtime/store/RelayReader.js b/packages/relay-runtime/store/RelayReader.js index 0255f91f4a7f8..1203841660247 100644 --- a/packages/relay-runtime/store/RelayReader.js +++ b/packages/relay-runtime/store/RelayReader.js @@ -68,7 +68,7 @@ const { } = require('./RelayStoreUtils'); const {NoopResolverCache} = require('./ResolverCache'); const { - RESOLVER_FRAGMENT_MISSING_DATA_SENTINEL, + RESOLVER_FRAGMENT_ERRORED_SENTINEL, withResolverContext, } = require('./ResolverFragments'); const {generateTypeID} = require('./TypeID'); @@ -653,6 +653,7 @@ class RelayReader { return { data: snapshot.data, isMissingData: snapshot.isMissingData, + missingRequiredFields: snapshot.missingRequiredFields, }; } @@ -665,6 +666,7 @@ class RelayReader { return { data: snapshot.data, isMissingData: snapshot.isMissingData, + missingRequiredFields: snapshot.missingRequiredFields, }; }; @@ -721,7 +723,7 @@ class RelayReader { getDataForResolverFragment, ); - this._propogateResolverMetadata( + this._propagateResolverMetadata( field.path, cachedSnapshot, resolverError, @@ -736,7 +738,7 @@ class RelayReader { // Reading a resolver field can uncover missing data, errors, suspense, // additional seen records and updated dataIDs. All of these facts must be // represented in the snapshot we return for this fragment. - _propogateResolverMetadata( + _propagateResolverMetadata( fieldPath: string, cachedSnapshot: ?Snapshot, resolverError: ?Error, @@ -1455,7 +1457,7 @@ function getResolverValue( resolverResult = resolverFunction.apply(null, resolverFunctionArgs); } catch (e) { - if (e === RESOLVER_FRAGMENT_MISSING_DATA_SENTINEL) { + if (e === RESOLVER_FRAGMENT_ERRORED_SENTINEL) { resolverResult = undefined; } else { resolverError = e; diff --git a/packages/relay-runtime/store/ResolverCache.js b/packages/relay-runtime/store/ResolverCache.js index db6653d62e32c..4bab1d35e86d9 100644 --- a/packages/relay-runtime/store/ResolverCache.js +++ b/packages/relay-runtime/store/ResolverCache.js @@ -11,6 +11,7 @@ 'use strict'; +import type {MissingRequiredFields} from '..'; import type { ReaderRelayLiveResolver, ReaderRelayResolver, @@ -51,6 +52,7 @@ export type EvaluationResult = { export type ResolverFragmentResult = { data: mixed, isMissingData: boolean, + missingRequiredFields: ?MissingRequiredFields, }; export type GetDataForResolverFragmentFn = diff --git a/packages/relay-runtime/store/ResolverFragments.js b/packages/relay-runtime/store/ResolverFragments.js index 78833ac240a12..90e39324c787c 100644 --- a/packages/relay-runtime/store/ResolverFragments.js +++ b/packages/relay-runtime/store/ResolverFragments.js @@ -111,22 +111,24 @@ function readFragment( fragmentSelector.kind === 'SingularReaderSelector', `Expected a singular reader selector for the fragment of the resolver ${fragmentNode.name}, but it was plural.`, ); - const {data, isMissingData} = context.getDataForResolverFragment( - fragmentSelector, - fragmentKey, - ); - - if (isMissingData) { - throw RESOLVER_FRAGMENT_MISSING_DATA_SENTINEL; + const {data, isMissingData, missingRequiredFields} = + context.getDataForResolverFragment(fragmentSelector, fragmentKey); + + if ( + isMissingData || + (missingRequiredFields != null && missingRequiredFields.action === 'THROW') + // TODO: Also consider @throwOnFieldError + ) { + throw RESOLVER_FRAGMENT_ERRORED_SENTINEL; } return data; } -const RESOLVER_FRAGMENT_MISSING_DATA_SENTINEL: mixed = {}; +const RESOLVER_FRAGMENT_ERRORED_SENTINEL: mixed = {}; module.exports = { readFragment, withResolverContext, - RESOLVER_FRAGMENT_MISSING_DATA_SENTINEL, + RESOLVER_FRAGMENT_ERRORED_SENTINEL, }; diff --git a/packages/relay-runtime/store/__tests__/RelayReader-Resolver-test.js b/packages/relay-runtime/store/__tests__/RelayReader-Resolver-test.js index 314268917623a..a953994b15a90 100644 --- a/packages/relay-runtime/store/__tests__/RelayReader-Resolver-test.js +++ b/packages/relay-runtime/store/__tests__/RelayReader-Resolver-test.js @@ -15,6 +15,9 @@ import type {Snapshot} from '../RelayStoreTypes'; const { constant_dependent: UserConstantDependentResolver, } = require('./resolvers/UserConstantDependentResolver'); +const { + requiredThrowNameCalls, +} = require('./resolvers/UserRequiredThrowNameResolver'); const invariant = require('invariant'); const nullthrows = require('nullthrows'); const {RelayFeatureFlags} = require('relay-runtime'); @@ -348,6 +351,50 @@ describe.each([true, false])( }); }); + it('propagates @required(action: THROW) errors from the resolver up to the reader and avoid calling resolver code', () => { + const source = RelayRecordSource.create({ + 'client:root': { + __id: 'client:root', + __typename: '__Root', + me: {__ref: '1'}, + }, + '1': { + __id: '1', + id: '1', + __typename: 'User', + name: null, // The missing field + }, + }); + const FooQuery = graphql` + query RelayReaderResolverTestRequiredThrowQuery { + me { + required_throw_name + } + } + `; + + const operation = createOperationDescriptor(FooQuery, {}); + const store = new RelayStore(source, {gcReleaseBufferSize: 0}); + + const beforeCallCount = requiredThrowNameCalls.count; + const {missingRequiredFields} = store.lookup(operation.fragment); + expect(requiredThrowNameCalls.count).toBe(beforeCallCount); + expect(missingRequiredFields).toEqual({ + action: 'THROW', + field: {owner: 'UserRequiredThrowNameResolver', path: 'name'}, + }); + + // Lookup a second time to ensure that we still report the missing fields when + // reading from the cache. + const {missingRequiredFields: missingRequiredFieldsTakeTwo} = + store.lookup(operation.fragment); + + expect(missingRequiredFieldsTakeTwo).toEqual({ + action: 'THROW', + field: {owner: 'UserRequiredThrowNameResolver', path: 'name'}, + }); + }); + it('works when the field is aliased', () => { const source = RelayRecordSource.create({ 'client:root': { diff --git a/packages/relay-runtime/store/__tests__/__generated__/RelayReaderResolverTestRequiredThrowQuery.graphql.js b/packages/relay-runtime/store/__tests__/__generated__/RelayReaderResolverTestRequiredThrowQuery.graphql.js new file mode 100644 index 0000000000000..8a1bb2b5f151e --- /dev/null +++ b/packages/relay-runtime/store/__tests__/__generated__/RelayReaderResolverTestRequiredThrowQuery.graphql.js @@ -0,0 +1,142 @@ +/** + * 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 { ConcreteRequest, Query } from 'relay-runtime'; +import type { UserRequiredThrowNameResolver$key } from "./../resolvers/__generated__/UserRequiredThrowNameResolver.graphql"; +import {required_throw_name as userRequiredThrowNameResolverType} from "../resolvers/UserRequiredThrowNameResolver.js"; +import type { TestResolverContextType } from "../../../mutations/__tests__/TestResolverContextType"; +// Type assertion validating that `userRequiredThrowNameResolverType` resolver is correctly implemented. +// A type error here indicates that the type signature of the resolver module is incorrect. +(userRequiredThrowNameResolverType: ( + rootKey: UserRequiredThrowNameResolver$key, + args: void, + context: TestResolverContextType, +) => ?string); +export type RelayReaderResolverTestRequiredThrowQuery$variables = {||}; +export type RelayReaderResolverTestRequiredThrowQuery$data = {| + +me: ?{| + +required_throw_name: ?string, + |}, +|}; +export type RelayReaderResolverTestRequiredThrowQuery = {| + response: RelayReaderResolverTestRequiredThrowQuery$data, + variables: RelayReaderResolverTestRequiredThrowQuery$variables, +|}; +*/ + +var node/*: ConcreteRequest*/ = { + "fragment": { + "argumentDefinitions": [], + "kind": "Fragment", + "metadata": null, + "name": "RelayReaderResolverTestRequiredThrowQuery", + "selections": [ + { + "alias": null, + "args": null, + "concreteType": "User", + "kind": "LinkedField", + "name": "me", + "plural": false, + "selections": [ + { + "alias": null, + "args": null, + "fragment": { + "args": null, + "kind": "FragmentSpread", + "name": "UserRequiredThrowNameResolver" + }, + "kind": "RelayResolver", + "name": "required_throw_name", + "resolverModule": require('./../resolvers/UserRequiredThrowNameResolver').required_throw_name, + "path": "me.required_throw_name" + } + ], + "storageKey": null + } + ], + "type": "Query", + "abstractKey": null + }, + "kind": "Request", + "operation": { + "argumentDefinitions": [], + "kind": "Operation", + "name": "RelayReaderResolverTestRequiredThrowQuery", + "selections": [ + { + "alias": null, + "args": null, + "concreteType": "User", + "kind": "LinkedField", + "name": "me", + "plural": false, + "selections": [ + { + "name": "required_throw_name", + "args": null, + "fragment": { + "kind": "InlineFragment", + "selections": [ + { + "alias": null, + "args": null, + "kind": "ScalarField", + "name": "name", + "storageKey": null + } + ], + "type": "User", + "abstractKey": null + }, + "kind": "RelayResolver", + "storageKey": null, + "isOutputType": true + }, + { + "alias": null, + "args": null, + "kind": "ScalarField", + "name": "id", + "storageKey": null + } + ], + "storageKey": null + } + ] + }, + "params": { + "cacheID": "bf7da4ab16e3296283aeca5290da4151", + "id": null, + "metadata": {}, + "name": "RelayReaderResolverTestRequiredThrowQuery", + "operationKind": "query", + "text": "query RelayReaderResolverTestRequiredThrowQuery {\n me {\n ...UserRequiredThrowNameResolver\n id\n }\n}\n\nfragment UserRequiredThrowNameResolver on User {\n name\n}\n" + } +}; + +if (__DEV__) { + (node/*: any*/).hash = "ac98b44e6541cf421098daaa34ca1e8d"; +} + +module.exports = ((node/*: any*/)/*: Query< + RelayReaderResolverTestRequiredThrowQuery$variables, + RelayReaderResolverTestRequiredThrowQuery$data, +>*/); diff --git a/packages/relay-runtime/store/__tests__/resolvers/UserRequiredThrowNameResolver.js b/packages/relay-runtime/store/__tests__/resolvers/UserRequiredThrowNameResolver.js new file mode 100644 index 0000000000000..2375ade9cdeca --- /dev/null +++ b/packages/relay-runtime/store/__tests__/resolvers/UserRequiredThrowNameResolver.js @@ -0,0 +1,50 @@ +/** + * 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 {UserRequiredNameResolver$key} from './__generated__/UserRequiredNameResolver.graphql'; + +const invariant = require('invariant'); +const {graphql} = require('relay-runtime'); +const {readFragment} = require('relay-runtime/store/ResolverFragments'); + +/** + * Represents the number of times the required_name resolver has been called + * and gotten past readFragment. + */ +const requiredThrowNameCalls: {count: number} = {count: 0}; + +/** + * @RelayResolver User.required_throw_name: String + * @rootFragment UserRequiredThrowNameResolver + */ +function required_name(rootKey: UserRequiredNameResolver$key): string { + const user = readFragment( + graphql` + fragment UserRequiredThrowNameResolver on User { + name @required(action: THROW) + } + `, + rootKey, + ); + requiredThrowNameCalls.count++; + invariant( + user != null, + 'This error should never throw because the @required should ensure this code never runs', + ); + return user.name; +} + +module.exports = { + required_name, + requiredThrowNameCalls, +}; diff --git a/packages/relay-runtime/store/__tests__/resolvers/__generated__/UserRequiredThrowNameResolver.graphql.js b/packages/relay-runtime/store/__tests__/resolvers/__generated__/UserRequiredThrowNameResolver.graphql.js new file mode 100644 index 0000000000000..0cffcf396e604 --- /dev/null +++ b/packages/relay-runtime/store/__tests__/resolvers/__generated__/UserRequiredThrowNameResolver.graphql.js @@ -0,0 +1,64 @@ +/** + * 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<<15edb01334a8a07f9e1e17e5b95c1349>> + * @flow + * @lightSyntaxTransform + * @nogrep + */ + +/* eslint-disable */ + +'use strict'; + +/*:: +import type { Fragment, ReaderFragment } from 'relay-runtime'; +import type { FragmentType } from "relay-runtime"; +declare export opaque type UserRequiredThrowNameResolver$fragmentType: FragmentType; +export type UserRequiredThrowNameResolver$data = {| + +name: string, + +$fragmentType: UserRequiredThrowNameResolver$fragmentType, +|}; +export type UserRequiredThrowNameResolver$key = { + +$data?: UserRequiredThrowNameResolver$data, + +$fragmentSpreads: UserRequiredThrowNameResolver$fragmentType, + ... +}; +*/ + +var node/*: ReaderFragment*/ = { + "argumentDefinitions": [], + "kind": "Fragment", + "metadata": null, + "name": "UserRequiredThrowNameResolver", + "selections": [ + { + "kind": "RequiredField", + "field": { + "alias": null, + "args": null, + "kind": "ScalarField", + "name": "name", + "storageKey": null + }, + "action": "THROW", + "path": "name" + } + ], + "type": "User", + "abstractKey": null +}; + +if (__DEV__) { + (node/*: any*/).hash = "add4c07fa95ec86cafaa371aba650cc8"; +} + +module.exports = ((node/*: any*/)/*: Fragment< + UserRequiredThrowNameResolver$fragmentType, + UserRequiredThrowNameResolver$data, +>*/);