Skip to content

Commit

Permalink
Mark field event handlers as handled (or not)
Browse files Browse the repository at this point in the history
Reviewed By: itamark

Differential Revision: D63565251

fbshipit-source-id: 43786afc5ae8ef491a5d04f85e53e7529e298523
  • Loading branch information
captbaritone authored and facebook-github-bot committed Sep 30, 2024
1 parent 535f35c commit 042ceca
Show file tree
Hide file tree
Showing 16 changed files with 80 additions and 3 deletions.
3 changes: 3 additions & 0 deletions packages/react-relay/__tests__/LiveResolvers-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -985,6 +985,7 @@ describe.each([true, false])(
kind: 'missing_required_field.throw',
owner: 'ResolverThatThrows',
fieldPath: 'username',
handled: false,
},
],
]);
Expand Down Expand Up @@ -1012,6 +1013,7 @@ describe.each([true, false])(
kind: 'relay_resolver.error',
owner: 'LiveResolversTest8Query',
shouldThrow: false,
handled: false,
},
],
]);
Expand Down Expand Up @@ -1715,6 +1717,7 @@ describe.each([true, false])(
fieldPath: 'live_resolver_throws',
error: new Error('What?'),
shouldThrow: false,
handled: false,
},
]);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,7 @@ describe.each([true, false])(
fieldPath: 'edge_to_model_that_throws.__relay_model_instance',
kind: 'relay_resolver.error',
shouldThrow: false,
handled: false,
},
]);
const data: $FlowExpectedError = snapshot.data;
Expand All @@ -533,6 +534,7 @@ describe.each([true, false])(
fieldPath: 'edge_to_plural_models_that_throw.__relay_model_instance',
kind: 'relay_resolver.error',
shouldThrow: false,
handled: false,
},
{
error: Error(ERROR_MESSAGE),
Expand All @@ -541,6 +543,7 @@ describe.each([true, false])(
fieldPath: 'edge_to_plural_models_that_throw.__relay_model_instance',
kind: 'relay_resolver.error',
shouldThrow: false,
handled: false,
},
]);
const data: $FlowExpectedError = snapshot.data;
Expand All @@ -567,6 +570,7 @@ describe.each([true, false])(
fieldPath: 'edge_to_plural_models_some_throw.__relay_model_instance',
kind: 'relay_resolver.error',
shouldThrow: false,
handled: false,
},
]);
const data: $FlowExpectedError = snapshot.data;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ describe('FragmentResource RelayResolver behavior', () => {
kind: 'relay_resolver.error',
owner: 'FragmentResourceResolverTestFragment1',
shouldThrow: false,
handled: false,
});
expect(event.error.message).toEqual('I always throw. What did you expect?');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ test('Throws if a @required(action: THROW) field is present and then goes missin
fieldPath: 'name',
kind: 'missing_required_field.throw',
owner: 'FragmentResourceRequiredFieldTestUserFragment',
handled: false,
});

disposable.dispose();
Expand Down
1 change: 1 addition & 0 deletions packages/relay-runtime/query/__tests__/fetchQuery-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ describe('fetchQuery with missing @required value', () => {
fieldPath: 'me.name',
kind: 'missing_required_field.throw',
owner: 'fetchQueryTest3Query',
handled: false,
});
expect(observer.error).toHaveBeenCalledWith(
Error(
Expand Down
10 changes: 9 additions & 1 deletion packages/relay-runtime/store/RelayReader.js
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ class RelayReader {
fieldPath: (error.path ?? []).join('.'),
error,
shouldThrow: this._selector.node.metadata?.throwOnFieldError ?? false,
handled: false,
});
}
}
Expand All @@ -227,7 +228,12 @@ class RelayReader {

this._errorResponseFields.push(
this._selector.node.metadata?.throwOnFieldError ?? false
? {kind: 'missing_expected_data.throw', owner, fieldPath}
? {
kind: 'missing_expected_data.throw',
owner,
fieldPath,
handled: false,
}
: {kind: 'missing_expected_data.log', owner, fieldPath},
);

Expand Down Expand Up @@ -292,6 +298,7 @@ class RelayReader {
kind: 'missing_required_field.throw',
fieldPath,
owner,
handled: false,
});
return;
case 'LOG':
Expand Down Expand Up @@ -782,6 +789,7 @@ class RelayReader {
shouldThrow:
this._selector.node.metadata?.throwOnFieldError ??
RelayFeatureFlags.ENABLE_FIELD_ERROR_HANDLING_THROW_BY_DEFAULT,
handled: false,
};
if (this._errorResponseFields == null) {
this._errorResponseFields = [errorEvent];
Expand Down
20 changes: 20 additions & 0 deletions packages/relay-runtime/store/RelayStoreTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -1284,11 +1284,16 @@ export type MissingExpectedDataLogEvent = {
*
* Relay will throw immediately after logging this event. If you wish to
* customize the error being thrown, you may throw your own error.
*
* *NOTE*: Only throw on this event if `handled` is false. Errors that have been
* handled by a `@catch` directive or by making a resolver null will have
* `handled: true` and should not trigger a throw.
*/
export type MissingExpectedDataThrowEvent = {
+kind: 'missing_expected_data.throw',
+owner: string,
+fieldPath: string,
+handled: boolean,
};

/**
Expand All @@ -1307,11 +1312,16 @@ export type MissingRequiredFieldLogEvent = {
*
* Relay will throw immediately after logging this event. If you wish to
* customize the error being thrown, you may throw your own error.
*
* *NOTE*: Only throw on this event if `handled` is false. Errors that have been
* handled by a `@catch` directive or by making a resolver null will have
* `handled: true` and should not trigger a throw.
*/
export type MissingRequiredFieldThrowEvent = {
+kind: 'missing_required_field.throw',
+owner: string,
+fieldPath: string,
+handled: boolean,
};

/**
Expand All @@ -1321,13 +1331,18 @@ export type MissingRequiredFieldThrowEvent = {
*
* If `@throwOnFieldError` was used on the parent query/fragment/mutation, you
* will also receive a TODO
*
* *NOTE*: Only throw on this event if `handled` is false. Errors that have been
* handled by a `@catch` directive or by making a resolver null will have
* `handled: true` and should not trigger a throw.
*/
export type RelayResolverErrorEvent = {
+kind: 'relay_resolver.error',
+owner: string,
+fieldPath: string,
+error: Error,
+shouldThrow: boolean,
+handled: boolean,
};

/**
Expand All @@ -1342,13 +1357,18 @@ export type RelayResolverErrorEvent = {
*
* https://relay.dev/docs/next/guides/catch-directive/
* https://relay.dev/docs/next/guides/throw-on-field-error-directive/
*
* *NOTE*: Only throw on this event if `handled` is false. Errors that have been
* handled by a `@catch` directive or by making a resolver null will have
* `handled: true` and should not trigger a throw.
*/
export type RelayFieldPayloadErrorEvent = {
+kind: 'relay_field_payload.error',
+owner: string,
+fieldPath: string,
+error: TRelayFieldError,
+shouldThrow: boolean,
+handled: boolean,
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ describe('RelayModernFragmentSpecResolver', () => {
fieldPath: 'name',
kind: 'missing_required_field.throw',
owner: 'RelayModernFragmentSpecResolverRequiredFieldTestUserFragment',
handled: false,
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ describe('RelayReader @catch', () => {
owner: 'RelayReaderCatchFieldsTestSiblingErrorQuery',
kind: 'relay_field_payload.error',
shouldThrow: false,
handled: false,
},
]);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ describe('RelayReader error fields', () => {
},
kind: 'relay_field_payload.error',
shouldThrow: false,
handled: false,
},
]);
});
Expand Down Expand Up @@ -112,11 +113,13 @@ describe('RelayReader error fields', () => {
path: ['me', 'lastName'],
},
shouldThrow: true,
handled: false,
},
{
owner: 'RelayReaderRelayErrorHandlingTest4Query',
fieldPath: '',
kind: 'missing_expected_data.throw',
handled: false,
},
]);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -965,6 +965,7 @@ describe('RelayReader @required', () => {
fieldPath: 'me.client_object',
kind: 'missing_required_field.throw',
owner: 'RelayReaderRequiredFieldsTest25Query',
handled: false,
},
]);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ describe.each([true, false])(
error: {message: 'There was an error!', path: ['me', 'lastName']},
owner: 'RelayReaderResolverTestFieldErrorQuery',
shouldThrow: false,
handled: false,
},
]);
});
Expand Down Expand Up @@ -258,6 +259,7 @@ describe.each([true, false])(
kind: 'relay_resolver.error',
owner: 'RelayReaderResolverTestRequiredQuery',
shouldThrow: false,
handled: false,
},
]);

Expand All @@ -278,6 +280,7 @@ describe.each([true, false])(
kind: 'relay_resolver.error',
owner: 'RelayReaderResolverTestRequiredQuery',
shouldThrow: false,
handled: false,
},
]);
});
Expand Down Expand Up @@ -356,6 +359,7 @@ describe.each([true, false])(
kind: 'relay_resolver.error',
owner: 'RelayReaderResolverTestRequiredWithParentQuery',
shouldThrow: false,
handled: false,
},
{
kind: 'missing_required_field.log',
Expand All @@ -381,6 +385,7 @@ describe.each([true, false])(
kind: 'relay_resolver.error',
owner: 'RelayReaderResolverTestRequiredWithParentQuery',
shouldThrow: false,
handled: false,
},
{
kind: 'missing_required_field.log',
Expand Down Expand Up @@ -423,6 +428,7 @@ describe.each([true, false])(
kind: 'missing_required_field.throw',
owner: 'UserRequiredThrowNameResolver',
fieldPath: 'name',
handled: false,
},
]);

Expand All @@ -436,6 +442,7 @@ describe.each([true, false])(
kind: 'missing_required_field.throw',
owner: 'UserRequiredThrowNameResolver',
fieldPath: 'name',
handled: false,
},
]);
});
Expand Down Expand Up @@ -1100,6 +1107,7 @@ describe.each([true, false])(
Object {
"error": [Error: I always throw. What did you expect?],
"fieldPath": "me.always_throws",
"handled": false,
"kind": "relay_resolver.error",
"owner": "RelayReaderResolverTest12Query",
"shouldThrow": false,
Expand All @@ -1120,6 +1128,7 @@ describe.each([true, false])(
Object {
"error": [Error: I always throw. What did you expect?],
"fieldPath": "me.always_throws",
"handled": false,
"kind": "relay_resolver.error",
"owner": "RelayReaderResolverTest12Query",
"shouldThrow": false,
Expand Down Expand Up @@ -1166,6 +1175,7 @@ describe.each([true, false])(
Object {
"error": [Error: I always throw. What did you expect?],
"fieldPath": "always_throws",
"handled": false,
"kind": "relay_resolver.error",
"owner": "UserAlwaysThrowsTransitivelyResolver",
"shouldThrow": false,
Expand All @@ -1186,6 +1196,7 @@ describe.each([true, false])(
Object {
"error": [Error: I always throw. What did you expect?],
"fieldPath": "always_throws",
"handled": false,
"kind": "relay_resolver.error",
"owner": "UserAlwaysThrowsTransitivelyResolver",
"shouldThrow": false,
Expand Down Expand Up @@ -1225,6 +1236,7 @@ describe.each([true, false])(
Object {
"error": [Error: Purposefully throwing before reading to exercise an edge case.],
"fieldPath": "throw_before_read",
"handled": false,
"kind": "relay_resolver.error",
"owner": "RelayReaderResolverTest14Query",
"shouldThrow": false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ test('Errors thrown during _initial_ read() are caught as resolver errors', () =
owner: 'LiveResolversTestHandlesErrorOnReadQuery',
fieldPath: 'counter_throws_when_odd',
shouldThrow: false,
handled: false,
},
]);
const data: $FlowExpectedError = snapshot.data;
Expand Down Expand Up @@ -321,6 +322,7 @@ test('Errors thrown during read() _after update_ are caught as resolver errors',
owner: 'LiveResolversTestHandlesErrorOnUpdateQuery',
fieldPath: 'counter_throws_when_odd',
shouldThrow: false,
handled: false,
},
]);
const nextData: $FlowExpectedError = nextSnapshot.data;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ test('Regular resolver with fragment reads live resovler with fragment', async (
fieldPath: 'counter',
kind: 'missing_required_field.throw',
owner: 'CounterPlusOneResolver',
handled: false,
},
]);
expect(recordIdsInStore).toEqual([
Expand Down
Loading

0 comments on commit 042ceca

Please sign in to comment.