Skip to content

Commit

Permalink
Avoid running resolver code if root fragment throws with @required(ac…
Browse files Browse the repository at this point in the history
…tion: THROW)
  • Loading branch information
captbaritone committed Sep 19, 2024
1 parent 0319d87 commit 3fb6775
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 17 deletions.
10 changes: 6 additions & 4 deletions packages/relay-runtime/store/RelayReader.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -653,6 +653,7 @@ class RelayReader {
return {
data: snapshot.data,
isMissingData: snapshot.isMissingData,
missingRequiredFields: snapshot.missingRequiredFields,
};
}

Expand All @@ -665,6 +666,7 @@ class RelayReader {
return {
data: snapshot.data,
isMissingData: snapshot.isMissingData,
missingRequiredFields: snapshot.missingRequiredFields,
};
};

Expand Down Expand Up @@ -721,7 +723,7 @@ class RelayReader {
getDataForResolverFragment,
);

this._propogateResolverMetadata(
this._propagateResolverMetadata(
field.path,
cachedSnapshot,
resolverError,
Expand All @@ -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,
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions packages/relay-runtime/store/ResolverCache.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

'use strict';

import type {MissingRequiredFields} from '..';
import type {
ReaderRelayLiveResolver,
ReaderRelayResolver,
Expand Down Expand Up @@ -51,6 +52,7 @@ export type EvaluationResult<T> = {
export type ResolverFragmentResult = {
data: mixed,
isMissingData: boolean,
missingRequiredFields: ?MissingRequiredFields,
};

export type GetDataForResolverFragmentFn =
Expand Down
20 changes: 11 additions & 9 deletions packages/relay-runtime/store/ResolverFragments.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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': {
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
};

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 3fb6775

Please sign in to comment.