Skip to content

Commit

Permalink
fix(federation): merge errors for shared root fields correctly (#6355)
Browse files Browse the repository at this point in the history
* fix(federation): merge errors for shared root fields correctly

* Tests

* aggregateerror message

* Spread AggregateError in errors in ExecutionResult

* Fix lint

---------

Co-authored-by: enisdenjo <[email protected]>
  • Loading branch information
ardatan and enisdenjo committed Jul 18, 2024
1 parent 97c88a0 commit c6d175b
Show file tree
Hide file tree
Showing 4 changed files with 232 additions and 8 deletions.
8 changes: 8 additions & 0 deletions .changeset/five-buttons-give.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@graphql-tools/federation': patch
---

Handle errors coming from subgraphs correctly when a root field is shared by different subgraphs

- If subgraph A returns an error for `Query.foo`, and subgraph B returns the data, ignore the error and keep it for null fields.
- If both subgraphs return errors, return them as `AggregateError` then return them to the gateway result.
45 changes: 39 additions & 6 deletions packages/executor/src/execution/execute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,11 @@ function executeImpl<TData = any, TVariables = any, TContext = any>(
throw exeContext.signal.reason;
}

exeContext.errors.push(error);
if (error.errors) {
exeContext.errors.push(...error.errors);
} else {
exeContext.errors.push(error);
}
return buildResponse<TData>(null, exeContext.errors);
},
)
Expand Down Expand Up @@ -691,6 +695,17 @@ function executeField(
// Note: we don't rely on a `catch` method, but we do expect "thenable"
// to take a second callback for the error case.
return completed.then(undefined, rawError => {
if (rawError instanceof AggregateError) {
return new AggregateError(
rawError.errors.map(rawErrorItem => {
rawErrorItem = coerceError(rawErrorItem);
const error = locatedError(rawErrorItem, fieldNodes, pathToArray(path));
const handledError = handleFieldError(error, returnType, errors);
filterSubsequentPayloads(exeContext, path, asyncPayloadRecord);
return handledError;
}),
);
}
rawError = coerceError(rawError);
const error = locatedError(rawError, fieldNodes, pathToArray(path));
const handledError = handleFieldError(error, returnType, errors);
Expand All @@ -700,6 +715,15 @@ function executeField(
}
return completed;
} catch (rawError) {
if (rawError instanceof AggregateError) {
return new AggregateError(
rawError.errors.map(rawErrorItem => {
const coercedError = coerceError(rawErrorItem);
const error = locatedError(coercedError, fieldNodes, pathToArray(path));
return handleFieldError(error, returnType, errors);
}),
);
}
const coercedError = coerceError(rawError);
const error = locatedError(coercedError, fieldNodes, pathToArray(path));
const handledError = handleFieldError(error, returnType, errors);
Expand Down Expand Up @@ -1615,16 +1639,25 @@ function mapSourceToResponse(
async (payload: unknown) =>
ensureAsyncIterable(await executeImpl(buildPerEventExecutionContext(exeContext, payload))),
(error: Error) => {
const wrappedError = createGraphQLError(error.message, {
originalError: error,
nodes: [exeContext.operation],
});
throw wrappedError;
if (error instanceof AggregateError) {
throw new AggregateError(
error.errors.map(e => wrapError(e, exeContext.operation)),
error.message,
);
}
throw wrapError(error, exeContext.operation);
},
),
);
}

function wrapError(error: Error, operation: OperationDefinitionNode) {
return createGraphQLError(error.message, {
originalError: error,
nodes: [operation],
});
}

function createSourceEventStreamImpl(
exeContext: ExecutionContext,
): MaybePromise<AsyncIterable<unknown> | SingularExecutionResult> {
Expand Down
25 changes: 23 additions & 2 deletions packages/federation/src/supergraph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1060,9 +1060,9 @@ export function getStitchingOptionsFromSupergraphSdl(
return jobs[0];
}
if (hasPromise) {
return Promise.all(jobs).then(results => mergeDeep(results, false, true, true));
return Promise.all(jobs).then(results => mergeResults(results));
}
return mergeDeep(jobs);
return mergeResults(jobs);
},
};
}
Expand Down Expand Up @@ -1219,3 +1219,24 @@ const entitiesFieldDefinitionNode: FieldDefinitionNode = {
};

const specifiedTypeNames = ['ID', 'String', 'Float', 'Int', 'Boolean', '_Any', '_Entity'];

function mergeResults(results: unknown[]) {
const errors: Error[] = [];
const datas: unknown[] = [];
for (const result of results) {
if (result instanceof AggregateError) {
errors.push(...result.errors);
} else if (result instanceof Error) {
errors.push(result);
} else {
datas.push(result);
}
}
if (datas.length) {
return mergeDeep(datas, false, true, true);
}
if (errors.length) {
return new AggregateError(errors, errors.map(error => error.message).join(', \n'));
}
return null;
}
162 changes: 162 additions & 0 deletions packages/federation/test/error-handling.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
import { GraphQLSchema, parse } from 'graphql';
import { IntrospectAndCompose, LocalGraphQLDataSource } from '@apollo/gateway';
import { buildSubgraphSchema } from '@apollo/subgraph';
import { createDefaultExecutor } from '@graphql-tools/delegate';
import { normalizedExecutor } from '@graphql-tools/executor';
import { isAsyncIterable } from '@graphql-tools/utils';
import { getStitchedSchemaFromSupergraphSdl } from '../src/supergraph';

describe('Error handling', () => {
let aResult: any;
let bResult: any;
const subgraphA = buildSubgraphSchema({
typeDefs: parse(/* GraphQL */ `
type Query {
foo: Foo
}
type Foo @key(fields: "id") {
id: ID!
bar: String
}
`),
resolvers: {
Query: {
foo() {
return aResult;
},
},
Foo: {
__resolveReference(root) {
return root;
},
bar() {
return 'Bar';
},
},
},
});
const subgraphB = buildSubgraphSchema({
typeDefs: parse(/* GraphQL */ `
type Query {
foo: Foo
}
extend type Foo @key(fields: "id") {
id: ID!
baz: String
}
`),
resolvers: {
Query: {
foo() {
return bResult;
},
},
Foo: {
__resolveReference(root) {
return root;
},
baz() {
return 'Baz';
},
},
},
});
let supergraph: GraphQLSchema;
beforeAll(async () => {
const { supergraphSdl } = await new IntrospectAndCompose({
subgraphs: [
{
name: 'A',
url: 'http://localhost:4001/graphql',
},
{
name: 'B',
url: 'http://localhost:4002/graphql',
},
],
}).initialize({
getDataSource({ name }) {
if (name === 'A') {
return new LocalGraphQLDataSource(subgraphA);
}
if (name === 'B') {
return new LocalGraphQLDataSource(subgraphB);
}
throw new Error(`Unknown subgraph: ${name}`);
},
async healthCheck() {},
update() {},
});
supergraph = getStitchedSchemaFromSupergraphSdl({
supergraphSdl,
onSubschemaConfig(subschemaConfig) {
if (subschemaConfig.name === 'A') {
subschemaConfig.executor = createDefaultExecutor(subgraphA);
} else if (subschemaConfig.name === 'B') {
subschemaConfig.executor = createDefaultExecutor(subgraphB);
} else {
throw new Error(`Unknown subgraph: ${subschemaConfig.name}`);
}
},
});
});
it('chooses the successful result from shared root fields', async () => {
aResult = new Error('A failed');
bResult = { id: '1' };
const result = await normalizedExecutor({
schema: supergraph,
document: parse(/* GraphQL */ `
query {
foo {
id
bar
baz
}
}
`),
});
if (isAsyncIterable(result)) {
throw new Error('Expected result to be an ExecutionResult');
}
expect(result.errors).toBeUndefined();
expect(result.data).toEqual({
foo: {
id: '1',
bar: null,
baz: 'Baz',
},
});
});
it('merges errors from shared root fields', async () => {
aResult = new Error('A failed');
bResult = new Error('B failed');
const result = await normalizedExecutor({
schema: supergraph,
document: parse(/* GraphQL */ `
query {
foo {
id
bar
baz
}
}
`),
});
if (isAsyncIterable(result)) {
throw new Error('Expected result to be an ExecutionResult');
}
expect(result.errors).toHaveLength(2);
expect(result.errors).toContainEqual(
expect.objectContaining({
message: 'A failed',
}),
);
expect(result.errors).toContainEqual(
expect.objectContaining({
message: 'B failed',
}),
);
});
});

0 comments on commit c6d175b

Please sign in to comment.