Unify resolver error type (#4796)

Summary: We had two types here, my larger goal is to unify each of these field error types to the point where RelayReader collects up a list of field events and they get passed to the logger (or throw) untransformed. Pull Request resolved: #4796 Reviewed By: tyao1 Differential Revision: D62787727 Pulled By: captbaritone fbshipit-source-id: 6e955d455e349a5a4111a5b3087ae2968848f812
facebook · Sep 18, 2024 · 0319d87 · 0319d87
1 parent 2f997ba
commit 0319d87
Show file tree

Hide file tree

Showing 8 changed files with 161 additions and 151 deletions.
diff --git a/packages/react-relay/__tests__/LiveResolvers-test.js b/packages/react-relay/__tests__/LiveResolvers-test.js
@@ -1709,10 +1709,9 @@ describe.each([true, false])(
       const data = environment.lookup(operation.fragment);
       expect(data.relayResolverErrors).toEqual([
         {
-          field: {
-            owner: 'LiveResolversTest18Query',
-            path: 'live_resolver_throws',
-          },
+          kind: 'relay_resolver.error',
+          owner: 'LiveResolversTest18Query',
+          fieldPath: 'live_resolver_throws',
           error: new Error('What?'),
         },
       ]);

diff --git a/packages/react-relay/__tests__/RelayResolverNullableModelClientEdge-test.js b/packages/react-relay/__tests__/RelayResolverNullableModelClientEdge-test.js
@@ -503,10 +503,9 @@ describe.each([true, false])(
       expect(snapshot.relayResolverErrors).toEqual([
         {
           error: Error(ERROR_MESSAGE),
-          field: {
-            owner: 'RelayResolverNullableModelClientEdgeTest_ErrorModel_Query',
-            path: 'edge_to_model_that_throws.__relay_model_instance',
-          },
+          kind: 'relay_resolver.error',
+          owner: 'RelayResolverNullableModelClientEdgeTest_ErrorModel_Query',
+          fieldPath: 'edge_to_model_that_throws.__relay_model_instance',
         },
       ]);
       const data: $FlowExpectedError = snapshot.data;
@@ -528,19 +527,17 @@ describe.each([true, false])(
       expect(snapshot.relayResolverErrors).toEqual([
         {
           error: Error(ERROR_MESSAGE),
-          field: {
-            owner:
-              'RelayResolverNullableModelClientEdgeTest_PluralErrorModel_Query',
-            path: 'edge_to_plural_models_that_throw.__relay_model_instance',
-          },
+          kind: 'relay_resolver.error',
+          owner:
+            'RelayResolverNullableModelClientEdgeTest_PluralErrorModel_Query',
+          fieldPath: 'edge_to_plural_models_that_throw.__relay_model_instance',
         },
         {
           error: Error(ERROR_MESSAGE),
-          field: {
-            owner:
-              'RelayResolverNullableModelClientEdgeTest_PluralErrorModel_Query',
-            path: 'edge_to_plural_models_that_throw.__relay_model_instance',
-          },
+          kind: 'relay_resolver.error',
+          owner:
+            'RelayResolverNullableModelClientEdgeTest_PluralErrorModel_Query',
+          fieldPath: 'edge_to_plural_models_that_throw.__relay_model_instance',
         },
       ]);
       const data: $FlowExpectedError = snapshot.data;
@@ -562,11 +559,10 @@ describe.each([true, false])(
       expect(snapshot.relayResolverErrors).toEqual([
         {
           error: Error(ERROR_MESSAGE),
-          field: {
-            owner:
-              'RelayResolverNullableModelClientEdgeTest_PluralSomeErrorModel_Query',
-            path: 'edge_to_plural_models_some_throw.__relay_model_instance',
-          },
+          kind: 'relay_resolver.error',
+          owner:
+            'RelayResolverNullableModelClientEdgeTest_PluralSomeErrorModel_Query',
+          fieldPath: 'edge_to_plural_models_some_throw.__relay_model_instance',
         },
       ]);
       const data: $FlowExpectedError = snapshot.data;

diff --git a/packages/relay-runtime/store/RelayReader.js b/packages/relay-runtime/store/RelayReader.js
@@ -355,7 +355,7 @@ class RelayReader {
       for (let i = 0; i < this._resolverErrors.length; i++) {
         const resolverError = this._resolverErrors[i];
         errors.push({
-          message: `Relay: Error in resolver for field at ${resolverError.field.path} in ${resolverError.field.owner}`,
+          message: `Relay: Error in resolver for field at ${resolverError.fieldPath} in ${resolverError.owner}`,
         });
       }
     }
@@ -776,7 +776,9 @@ class RelayReader {
     // to be logged.
     if (resolverError) {
       this._resolverErrors.push({
-        field: {path: fieldPath, owner: this._fragmentName},
+        kind: 'relay_resolver.error',
+        fieldPath,
+        owner: this._fragmentName,
         error: resolverError,
       });
     }

diff --git a/packages/relay-runtime/store/RelayStoreTypes.js b/packages/relay-runtime/store/RelayStoreTypes.js
@@ -149,12 +149,7 @@ export type MissingClientEdgeRequestInfo = {
   +clientEdgeDestinationID: DataID,
 };
 
-export type RelayResolverError = {
-  field: FieldLocation,
-  error: Error,
-};
-
-export type RelayResolverErrors = Array<RelayResolverError>;
+export type RelayResolverErrors = Array<RelayResolverErrorEvent>;
 
 export type MissingLiveResolverField = {
   +path: string,
@@ -1260,95 +1255,121 @@ export type MissingFieldHandler =
       ) => ?Array<?DataID>,
     };
 
-export type RelayFieldLoggerEvent =
-  /**
-   * Data which Relay expected to be in the store (because it was requested by
-   * the parent query/mutation/subscription) was missing. This can happen due
-   * to graph relationship changes observed by other queries/mutations, or
-   * imperative updates that don't provide all needed data.
-   *
-   * https://relay.dev/docs/next/debugging/why-null/#graph-relationship-change
-   *
-   * In this case Relay will render with the referenced field as `undefined`.
-   *
-   * __NOTE__: This may break with the type contract of Relay's generated types.
-   *
-   * To turn this into a hard error for a given fragment/query, you can use
-   * `@thowOnFieldError`.
-   *
-   * https://relay.dev/docs/next/guides/throw-on-field-error-directive/
-   */
-  | {+kind: 'missing_expected_data.log', +owner: string, +fieldPath: string}
+/**
+ * Data which Relay expected to be in the store (because it was requested by
+ * the parent query/mutation/subscription) was missing. This can happen due
+ * to graph relationship changes observed by other queries/mutations, or
+ * imperative updates that don't provide all needed data.
+ *
+ * https://relay.dev/docs/next/debugging/why-null/#graph-relationship-change
+ *
+ * In this case Relay will render with the referenced field as `undefined`.
+ *
+ * __NOTE__: This may break with the type contract of Relay's generated types.
+ *
+ * To turn this into a hard error for a given fragment/query, you can use
+ * `@throwOnFieldError`.
+ *
+ * https://relay.dev/docs/next/guides/throw-on-field-error-directive/
+ */
+export type MissingExpectedDataLogEvent = {
+  +kind: 'missing_expected_data.log',
+  +owner: string,
+  +fieldPath: string,
+};
 
-  /**
-   * Data which Relay expected to be in the store (because it was requested by
-   * the parent query/mutation/subscription) was missing. This can happen due
-   * to graph relationship changes observed by other queries/mutations, or
-   * imperative updates that don't provide all needed data.
-   *
-   * https://relay.dev/docs/next/debugging/why-null/#graph-relationship-change
-   *
-   * This event is as `.throw` because the missing data was encountered in a
-   * query/fragment/mutaiton with `@throwOnFieldError` `@thowOnFieldError`.
-   *
-   * https://relay.dev/docs/next/guides/throw-on-field-error-directive/
-   *
-   * Relay will throw immediately after logging this event. If you wish to
-   * customize the error being thrown, you may throw your own error.
-   */
-  | {+kind: 'missing_expected_data.throw', +owner: string, +fieldPath: string}
+/**
+ * Data which Relay expected to be in the store (because it was requested by
+ * the parent query/mutation/subscription) was missing. This can happen due
+ * to graph relationship changes observed by other queries/mutations, or
+ * imperative updates that don't provide all needed data.
+ *
+ * https://relay.dev/docs/next/debugging/why-null/#graph-relationship-change
+ *
+ * This event is as `.throw` because the missing data was encountered in a
+ * query/fragment/mutation with `@throwOnFieldError` `@throwOnFieldError`.
+ *
+ * https://relay.dev/docs/next/guides/throw-on-field-error-directive/
+ *
+ * Relay will throw immediately after logging this event. If you wish to
+ * customize the error being thrown, you may throw your own error.
+ */
+export type MissingExpectedDataThrowEvent = {
+  +kind: 'missing_expected_data.throw',
+  +owner: string,
+  +fieldPath: string,
+};
 
-  /**
-   * A field was marked as @required(action: LOG) but was null or missing in the
-   * store.
-   */
-  | {+kind: 'missing_required_field.log', +owner: string, +fieldPath: string}
+/**
+ * A field was marked as @required(action: LOG) but was null or missing in the
+ * store.
+ */
+export type MissingFieldLogEvent = {
+  +kind: 'missing_required_field.log',
+  +owner: string,
+  +fieldPath: string,
+};
 
-  /**
-   * A field was marked as @required(action: THROW) but was null or missing in the
-   * store.
-   *
-   * Relay will throw immediately after logging this event. If you wish to
-   * customize the error being thrown, you may throw your own error.
-   */
-  | {+kind: 'missing_required_field.throw', +owner: string, +fieldPath: string}
+/**
+ * A field was marked as @required(action: THROW) but was null or missing in the
+ * store.
+ *
+ * Relay will throw immediately after logging this event. If you wish to
+ * customize the error being thrown, you may throw your own error.
+ */
+export type MissingFieldThrowEvent = {
+  +kind: 'missing_required_field.throw',
+  +owner: string,
+  +fieldPath: string,
+};
 
-  /**
-   * A Relay Resolver that is currently being read threw a JavaScript error when
-   * it was last evaluated. By default, the value has been coerced to null and
-   * passed to the product code.
-   *
-   * If `@throwOnFieldError` was used on the parent query/fragment/mutation, you
-   * will also recieve a TODO
-   */
-  | {
-      +kind: 'relay_resolver.error',
-      +owner: string,
-      +fieldPath: string,
-      +error: Error,
-    }
+/**
+ * A Relay Resolver that is currently being read threw a JavaScript error when
+ * it was last evaluated. By default, the value has been coerced to null and
+ * passed to the product code.
+ *
+ * If `@throwOnFieldError` was used on the parent query/fragment/mutation, you
+ * will also receive a TODO
+ */
+export type RelayResolverErrorEvent = {
+  +kind: 'relay_resolver.error',
+  +owner: string,
+  +fieldPath: string,
+  +error: Error,
+};
 
-  /**
-   * A field being read by Relay was marked as being in an error state by the
-   * GraphQL response.
-   *
-   * https://spec.graphql.org/October2021/#sec-Errors.Field-errors
-   *
-   * If the field's parent query/fragment/mutation was annotated with
-   * `@throwOnFieldError` and no `@catch` directive was used to catch the error,
-   * Relay will throw an error immediately after logging this event.
-   *
-   * https://relay.dev/docs/next/guides/catch-directive/
-   * https://relay.dev/docs/next/guides/throw-on-field-error-directive/
-   */
-  | {
-      +kind: 'relay_field_payload.error',
-      +owner: string,
-      +fieldPath: string,
-      +error: TRelayFieldError,
-      // TODO: Should we add `shouldThrow` as a flag here, or perhaps have two
-      // different event types?
-    };
+/**
+ * A field being read by Relay was marked as being in an error state by the
+ * GraphQL response.
+ *
+ * https://spec.graphql.org/October2021/#sec-Errors.Field-errors
+ *
+ * If the field's parent query/fragment/mutation was annotated with
+ * `@throwOnFieldError` and no `@catch` directive was used to catch the error,
+ * Relay will throw an error immediately after logging this event.
+ *
+ * https://relay.dev/docs/next/guides/catch-directive/
+ * https://relay.dev/docs/next/guides/throw-on-field-error-directive/
+ */
+export type RelayFieldPayloadErrorEvent = {
+  +kind: 'relay_field_payload.error',
+  +owner: string,
+  +fieldPath: string,
+  +error: TRelayFieldError,
+  // TODO: Should we add `shouldThrow` as a flag here, or perhaps have two
+  // different event types?
+};
+
+/**
+ * Union of all RelayFieldLoggerEvent types
+ */
+export type RelayFieldLoggerEvent =
+  | MissingExpectedDataLogEvent
+  | MissingExpectedDataThrowEvent
+  | MissingFieldLogEvent
+  | MissingFieldThrowEvent
+  | RelayResolverErrorEvent
+  | RelayFieldPayloadErrorEvent;
 
 /**
  * A handler for events related to @required fields. Currently reports missing

diff --git a/packages/relay-runtime/store/__tests__/RelayReader-Resolver-test.js b/packages/relay-runtime/store/__tests__/RelayReader-Resolver-test.js
@@ -1007,10 +1007,9 @@ describe.each([true, false])(
       Array [
         Object {
           "error": [Error: I always throw. What did you expect?],
-          "field": Object {
-            "owner": "RelayReaderResolverTest12Query",
-            "path": "me.always_throws",
-          },
+          "fieldPath": "me.always_throws",
+          "kind": "relay_resolver.error",
+          "owner": "RelayReaderResolverTest12Query",
         },
       ]
     `);
@@ -1027,10 +1026,9 @@ describe.each([true, false])(
       Array [
         Object {
           "error": [Error: I always throw. What did you expect?],
-          "field": Object {
-            "owner": "RelayReaderResolverTest12Query",
-            "path": "me.always_throws",
-          },
+          "fieldPath": "me.always_throws",
+          "kind": "relay_resolver.error",
+          "owner": "RelayReaderResolverTest12Query",
         },
       ]
     `);
@@ -1073,10 +1071,9 @@ describe.each([true, false])(
       Array [
         Object {
           "error": [Error: I always throw. What did you expect?],
-          "field": Object {
-            "owner": "UserAlwaysThrowsTransitivelyResolver",
-            "path": "always_throws",
-          },
+          "fieldPath": "always_throws",
+          "kind": "relay_resolver.error",
+          "owner": "UserAlwaysThrowsTransitivelyResolver",
         },
       ]
     `);
@@ -1093,10 +1090,9 @@ describe.each([true, false])(
       Array [
         Object {
           "error": [Error: I always throw. What did you expect?],
-          "field": Object {
-            "owner": "UserAlwaysThrowsTransitivelyResolver",
-            "path": "always_throws",
-          },
+          "fieldPath": "always_throws",
+          "kind": "relay_resolver.error",
+          "owner": "UserAlwaysThrowsTransitivelyResolver",
         },
       ]
     `);
@@ -1132,10 +1128,9 @@ describe.each([true, false])(
       Array [
         Object {
           "error": [Error: Purposefully throwing before reading to exercise an edge case.],
-          "field": Object {
-            "owner": "RelayReaderResolverTest14Query",
-            "path": "throw_before_read",
-          },
+          "fieldPath": "throw_before_read",
+          "kind": "relay_resolver.error",
+          "owner": "RelayReaderResolverTest14Query",
         },
       ]
     `);