Avoid running resolver code if root fragment throws with @required(action: THROW)

Author: captbaritone

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;

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<T> = {
 export type ResolverFragmentResult = {
   data: mixed,
   isMissingData: boolean,
+  missingRequiredFields: ?MissingRequiredFields,
 };
 
 export type GetDataForResolverFragmentFn =

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,
 };

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': {

packages/relay-runtime/store/__tests__/__generated__/RelayReaderResolverTestRequiredThrowQuery.graphql.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,
+};