Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(gateway): Remove path, query and variables field from subgraph error responses #900

Merged
merged 5 commits into from
Jul 21, 2021
Merged

fix(gateway): Remove path, query and variables field from subgraph error responses #900

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
af4d9e6
Remove query and variables from downstream subgraph errors
trevor-scheer Jul 21, 2021
07411c3
Update gateway-js/CHANGELOG.md
trevor-scheer Jul 21, 2021
08cfba7
Update gateway-js/CHANGELOG.md
trevor-scheer Jul 21, 2021
d9e2322
Hide path as well
trevor-scheer Jul 21, 2021
acaf0b1
Update changelog with feedback from @abernix
trevor-scheer Jul 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion gateway-js/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@

- Change default managed federation mechanism over to use Apollo's new Uplink service. This service handles composition and sends the entire supergraph to the gateway. Previously the gateway was responsible for downloading each service's SDL from GCS and handling the composition itself. If you have any issues trying to use this new behavior, you may use the gateway config option `schemaConfigDeliveryEndpoint: null` to continue using the previous mechanism for the time being. If you were previously setting the `experimental_schemaConfigDeliveryEndpoint` config option, you will need to update the name of the option itself (or you can remove it entirely if you were using Apollo's Uplink service). [PR #881](https://github.com/apollographql/federation/pull/881)
- Introduce support for removing @inaccessible elements from the API schema. [PR #807](https://github.com/apollographql/federation/pull/859)
- Call `toAPISchema` within the try/catch block in `loadStatic`. [PR 894](https://github.com/apollographql/federation/pull/894)
- Call `toAPISchema` within the try/catch block in `loadStatic`. [PR #894](https://github.com/apollographql/federation/pull/894)
- Remove `query` and `variables` from downstream subgraph error responses. [PR #900](https://github.com/apollographql/federation/pull/900)
trevor-scheer marked this conversation as resolved.
Show resolved Hide resolved

## v0.33.0

Expand Down
7 changes: 2 additions & 5 deletions gateway-js/src/__tests__/executeQueryPlan.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,8 @@ describe('executeQueryPlan', () => {
'errors.0.extensions.serviceName',
'accounts',
);
expect(response).toHaveProperty(
'errors.0.extensions.query',
'{me{name{first last}}}',
);
expect(response).toHaveProperty('errors.0.extensions.variables', {});
expect(response).not.toHaveProperty('errors.0.extensions.query');
expect(response).not.toHaveProperty('errors.0.extensions.variables');
});

it(`should not send request to downstream services when all entities are undefined`, async () => {
Expand Down
19 changes: 3 additions & 16 deletions gateway-js/src/executeQueryPlan.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,13 +383,8 @@ async function executeFetch<TContext>(
});

if (response.errors) {
const errors = response.errors.map(error =>
downstreamServiceError(
error,
fetch.serviceName,
source,
variables,
),
const errors = response.errors.map((error) =>
downstreamServiceError(error, fetch.serviceName),
);
context.errors.push(...errors);
}
Expand Down Expand Up @@ -551,14 +546,8 @@ function flattenResultsAtPath(value: any, path: ResponsePath): any {
function downstreamServiceError(
originalError: GraphQLFormattedError,
serviceName: string,
query: string,
variables?: Record<string, any>,
) {
let {
message,
extensions,
path,
} = originalError
let { message, extensions, path } = originalError;
trevor-scheer marked this conversation as resolved.
Show resolved Hide resolved

if (!message) {
message = `Error while fetching subquery from service "${serviceName}"`;
Expand All @@ -568,8 +557,6 @@ function downstreamServiceError(
// XXX The presence of a serviceName in extensions is used to
// determine if this error should be captured for metrics reporting.
serviceName,
query,
variables,
...extensions,
};
return new GraphQLError(
Expand Down