Support marking field errors as handled

Reviewed By: itamark

Differential Revision: D63567893

fbshipit-source-id: 8a4021a10043b85803f10f26c204303d18d2bd19

packages/react-relay/__tests__/LiveResolvers-test.js

@@ -970,22 +970,25 @@ describe.each([true, false])(
         const environment = createEnvironment(source);
 
         // First, render with missing data
-        await expect(async () => {
-          await TestRenderer.act(() => {
-            TestRenderer.create(
-              <TestComponent environment={environment} id="1" />,
-            );
-          });
-        }).rejects.toThrow(
-          "Relay: Missing @required value at path 'username' in 'ResolverThatThrows'.",
-        );
+        await TestRenderer.act(() => {
+          TestRenderer.create(
+            <TestComponent environment={environment} id="1" />,
+          );
+        });
         expect(relayFieldLogger.mock.calls).toEqual([
+          [
+            {
+              fieldPath: '',
+              kind: 'missing_expected_data.log',
+              owner: 'ResolverThatThrows',
+            },
+          ],
           [
             {
               kind: 'missing_required_field.throw',
               owner: 'ResolverThatThrows',
               fieldPath: 'username',
-              handled: false,
+              handled: true,
             },
           ],
         ]);

packages/relay-runtime/index.js

@@ -64,7 +64,9 @@ const getRefetchMetadata = require('./util/getRefetchMetadata');
 const getRelayHandleKey = require('./util/getRelayHandleKey');
 const getRequestIdentifier = require('./util/getRequestIdentifier');
 const getValueAtPath = require('./util/getValueAtPath');
-const handlePotentialSnapshotErrors = require('./util/handlePotentialSnapshotErrors');
+const {
+  handlePotentialSnapshotErrors,
+} = require('./util/handlePotentialSnapshotErrors');
 const isPromise = require('./util/isPromise');
 const isScalarAndEqual = require('./util/isScalarAndEqual');
 const recycleNodesInto = require('./util/recycleNodesInto');

packages/relay-runtime/query/fetchQuery.js

@@ -28,7 +28,9 @@ const RelayObservable = require('../network/RelayObservable');
 const {
   createOperationDescriptor,
 } = require('../store/RelayModernOperationDescriptor');
-const handlePotentialSnapshotErrors = require('../util/handlePotentialSnapshotErrors');
+const {
+  handlePotentialSnapshotErrors,
+} = require('../util/handlePotentialSnapshotErrors');
 const fetchQueryInternal = require('./fetchQueryInternal');
 const {getRequest} = require('./GraphQLTag');
 const invariant = require('invariant');

packages/relay-runtime/store/RelayModernFragmentSpecResolver.js

@@ -27,7 +27,9 @@ import type {
 } from './RelayStoreTypes';
 
 const getPendingOperationsForFragment = require('../util/getPendingOperationsForFragment');
-const handlePotentialSnapshotErrors = require('../util/handlePotentialSnapshotErrors');
+const {
+  handlePotentialSnapshotErrors,
+} = require('../util/handlePotentialSnapshotErrors');
 const isScalarAndEqual = require('../util/isScalarAndEqual');
 const recycleNodesInto = require('../util/recycleNodesInto');
 const RelayFeatureFlags = require('../util/RelayFeatureFlags');

packages/relay-runtime/store/RelayReader.js

@@ -32,6 +32,7 @@ import type {DataID, Variables} from '../util/RelayRuntimeTypes';
 import type {
   ClientEdgeTraversalInfo,
   DataIDSet,
+  ErrorResponseField,
   ErrorResponseFields,
   MissingClientEdgeRequestInfo,
   MissingLiveResolverField,
@@ -329,6 +330,7 @@ class RelayReader {
       "Couldn't determine field name for this field. It might be a ReaderClientExtension - which is not yet supported.",
     );
 
+    // TODO: Should we be hiding log level events here?
     const errors = this._errorResponseFields
       ?.map(error => {
         switch (error.kind) {
@@ -422,18 +424,17 @@ class RelayReader {
 
           this._errorResponseFields = previousResponseFields;
 
-          // If we encountered non-throwing @required fields, in the children,
-          // we want to preserve those errors in the snapshot.
           if (childrenErrorResponseFields != null) {
+            if (this._errorResponseFields == null) {
+              this._errorResponseFields = [];
+            }
             for (let i = 0; i < childrenErrorResponseFields.length; i++) {
-              const event = childrenErrorResponseFields[i];
-              if (event.kind === 'missing_required_field.log') {
-                if (this._errorResponseFields == null) {
-                  this._errorResponseFields = [event];
-                } else {
-                  this._errorResponseFields.push(event);
-                }
-              }
+              // We mark any errors encountered within the @catch as "handled"
+              // to ensure that they don't cause the reader to throw, but can
+              // still be logged.
+              this._errorResponseFields.push(
+                markFieldErrorHasHandled(childrenErrorResponseFields[i]),
+              );
             }
           }
 
@@ -762,15 +763,18 @@ class RelayReader {
           this._errorResponseFields = [];
         }
         for (const error of cachedSnapshot.errorResponseFields) {
-          // TODO: In reality we should propagate _all_ errors, but
-          // for now we're only propagating resolver errors and missing field
-          // errors for backwards compatibility with previous behavior.
-          if (
-            error.kind === 'relay_resolver.error' ||
-            error.kind === 'missing_required_field.throw' ||
-            error.kind === 'missing_required_field.log'
-          ) {
+          if (this._selector.node.metadata?.throwOnFieldError === true) {
+            // If this fragment is @throwOnFieldError, any destructive error
+            // encountered inside a resolver's fragment is equivilent to the
+            // resolver field having a field error, and we want that to cause this
+            // fragment to throw. So, we propagate all errors as is.
             this._errorResponseFields.push(error);
+          } else {
+            // If this fragment is _not_ @throwOnFieldError, we will simply
+            // accept that any destructive errors encountered in the resolver's
+            // root fragment will cause the resolver to return null, and well
+            // pass the errors along to the logger marked as "handled".
+            this._errorResponseFields.push(markFieldErrorHasHandled(error));
           }
         }
       }
@@ -1413,6 +1417,24 @@ class RelayReader {
   }
 }
 
+function markFieldErrorHasHandled(
+  event: ErrorResponseField,
+): ErrorResponseField {
+  switch (event.kind) {
+    case 'missing_expected_data.throw':
+    case 'missing_required_field.throw':
+    case 'relay_field_payload.error':
+    case 'relay_resolver.error':
+      return {...event, handled: true};
+    case 'missing_expected_data.log':
+    case 'missing_required_field.log':
+      return event;
+    default:
+      event.kind as empty;
+      invariant(false, 'Unexpected error response field kind: %s', event.kind);
+  }
+}
+
 // Constructs the arguments for a resolver function and then evaluates it.
 //
 // If the resolver's fragment is missing data (query is in-flight, a dependency
@@ -1450,9 +1472,10 @@ function getResolverValue(
 
     resolverResult = resolverFunction.apply(null, resolverFunctionArgs);
   } catch (e) {
-    if (e === RESOLVER_FRAGMENT_ERRORED_SENTINEL) {
-      resolverResult = undefined;
-    } else {
+    // GraphQL coerces resolver errors to null or nullable fields, and Relay
+    // does not support non-nullable Relay Resolvers.
+    resolverResult = null;
+    if (e !== RESOLVER_FRAGMENT_ERRORED_SENTINEL) {
       resolverError = e;
     }
   }

packages/relay-runtime/store/ResolverFragments.js

@@ -17,6 +17,7 @@ import type {FragmentType, SingularReaderSelector} from './RelayStoreTypes';
 import type {ResolverFragmentResult} from './ResolverCache';
 
 const {getFragment} = require('../query/GraphQLTag');
+const {eventShouldThrow} = require('../util/handlePotentialSnapshotErrors');
 const {getSelector} = require('./RelayModernSelector');
 const invariant = require('invariant');
 
@@ -116,11 +117,7 @@ function readFragment(
 
   if (
     isMissingData ||
-    (errorResponseFields != null &&
-      errorResponseFields.some(
-        // TODO: Also consider @throwOnFieldError
-        event => event.kind === 'missing_required_field.throw',
-      ))
+    (errorResponseFields != null && errorResponseFields.some(eventShouldThrow))
   ) {
     throw RESOLVER_FRAGMENT_ERRORED_SENTINEL;
   }

packages/relay-runtime/store/__tests__/RelayReader-CatchFields-test.js

@@ -90,7 +90,16 @@ describe('RelayReader @catch', () => {
         },
       });
 
-      expect(errorResponseFields).toEqual(null);
+      expect(errorResponseFields).toEqual([
+        {
+          error: {message: 'There was an error!', path: ['me', 'lastName']},
+          fieldPath: 'me.lastName',
+          handled: true,
+          kind: 'relay_field_payload.error',
+          owner: 'RelayReaderCatchFieldsTest01Query',
+          shouldThrow: false,
+        },
+      ]);
     });
 
     it('if preceding scalar sibling has error, catch to RESULT should not catch that error', () => {
@@ -201,6 +210,12 @@ describe('RelayReader @catch', () => {
           fieldPath: 'alsoMe.lastName',
           owner: 'RelayReaderCatchFieldsTestSiblingLogRequiredErrorQuery',
         },
+        {
+          fieldPath: 'me.firstName',
+          handled: true,
+          kind: 'missing_required_field.throw',
+          owner: 'RelayReaderCatchFieldsTestSiblingLogRequiredErrorQuery',
+        },
       ]);
     });
 
@@ -232,7 +247,14 @@ describe('RelayReader @catch', () => {
         me: null,
       });
 
-      expect(errorResponseFields).toEqual(null);
+      expect(errorResponseFields).toEqual([
+        {
+          fieldPath: 'me.firstName',
+          handled: true,
+          kind: 'missing_required_field.throw',
+          owner: 'RelayReaderCatchFieldsTestRequiredCatchToNullErrorQuery',
+        },
+      ]);
     });
 
     it('@catch(to: NULL) catching missing data returns null', () => {
@@ -273,7 +295,13 @@ describe('RelayReader @catch', () => {
       // We still need to ensure that we will suspend if there is a request in flight.
       expect(isMissingData).toEqual(true);
 
-      expect(errorResponseFields).toEqual(null);
+      expect(errorResponseFields).toEqual([
+        {
+          fieldPath: '',
+          kind: 'missing_expected_data.log',
+          owner: 'RelayReaderCatchFieldsTestCatchMissingToNullErrorQuery',
+        },
+      ]);
     });
 
     it('if scalar has catch to RESULT - but no error, response should reflect', () => {
@@ -391,7 +419,16 @@ describe('RelayReader @catch', () => {
         },
       });
 
-      expect(errorResponseFields).toEqual(null);
+      expect(errorResponseFields).toEqual([
+        {
+          error: {message: 'There was an error!', path: ['me', 'lastName']},
+          fieldPath: 'me.lastName',
+          handled: true,
+          kind: 'relay_field_payload.error',
+          owner: 'RelayReaderCatchFieldsTest07Query',
+          shouldThrow: false,
+        },
+      ]);
     });
 
     it('if scalar has catch to RESULT with nested required', () => {
@@ -430,7 +467,14 @@ describe('RelayReader @catch', () => {
         },
       });
 
-      expect(errorResponseFields).toEqual(null);
+      expect(errorResponseFields).toEqual([
+        {
+          fieldPath: 'me.lastName',
+          handled: true,
+          kind: 'missing_required_field.throw',
+          owner: 'RelayReaderCatchFieldsTest02Query',
+        },
+      ]);
     });
   });
 });