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.

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

gateway RemoteGraphQLDataSource: throw GraphQLError, not ApolloError #2028

Merged
merged 1 commit into from Aug 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 4 additions & 2 deletions gateway-js/CHANGELOG.md
Expand Up @@ -2,9 +2,11 @@

This CHANGELOG pertains only to Apollo Federation packages in the 2.x range. The Federation v0.x equivalent for this package can be found [here](https://github.com/apollographql/federation/blob/version-0.x/gateway-js/CHANGELOG.md) on the `version-0.x` branch of this repo.

- Fix fragment reuse in subgraph fetches [PR #1911](https://github.com/apollographql/federation/pull/1911).
- Allow passing a custom `fetcher` [PR 1997#](https://github.com/apollographql/federation/pull/1997).
- Fix fragment reuse in subgraph fetches [PR #1911](https://github.com/apollographql/federation/pull/1911).
- Allow passing a custom `fetcher` [PR #1997](https://github.com/apollographql/federation/pull/1997).
- __UNBREAKING__: Previous 2.1.0 alphas removed the custom fetcher for Apollo Uplink. This re-adds that parameter, and requires the fetcher to have the `AbortSignal` interface https://fetch.spec.whatwg.org/#requestinit.
- The method `RemoteGraphQLDataSource.errorFromResponse` now returns a `GraphQLError` (as defined by `graphql`) rather than an `ApolloError` (as defined by `apollo-server-errors`). [PR #2028](https://github.com/apollographql/federation/pull/2028)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion Would you mind adding a __BREAKING__ sub-bullet to this about the change in return type? I agree it's probably low-impact but might as well be explicit.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also tweaked some formatting in some of the lines above

- __BREAKING__: If you call `RemoteGraphQLDataSource.errorFromResponse` manually and expect its return value to be a particular subclass of `GraphQLError`, or if you expect the error received by `didEncounterError` to be a particular subclass of `GraphQLError`, then this change may affect you. We recommend checking `error.extensions.code` instead.

## 2.1.0-alpha.1

Expand Down
1 change: 0 additions & 1 deletion gateway-js/package.json
Expand Up @@ -37,7 +37,6 @@
"@opentelemetry/api": "^1.0.1",
"apollo-reporting-protobuf": "^0.8.0 || ^3.0.0",
"apollo-server-core": "^2.23.0 || ^3.0.0",
"apollo-server-errors": "^2.5.0 || ^3.0.0",
"apollo-server-types": "^0.9.0 || ^3.0.0",
"async-retry": "^1.3.3",
"loglevel": "^1.6.1",
Expand Down
31 changes: 12 additions & 19 deletions gateway-js/src/datasources/RemoteGraphQLDataSource.ts
Expand Up @@ -7,18 +7,14 @@ import {
CacheScope,
CachePolicy,
} from 'apollo-server-types';
import {
ApolloError,
AuthenticationError,
ForbiddenError,
} from 'apollo-server-errors';
import { isObject } from '../utilities/predicates';
import { GraphQLDataSource, GraphQLDataSourceProcessOptions, GraphQLDataSourceRequestKind } from './types';
import { createHash } from '@apollo/utils.createhash';
import { parseCacheControlHeader } from './parseCacheControlHeader';
import fetcher from 'make-fetch-happen';
import { Headers as NodeFetchHeaders, Request as NodeFetchRequest } from 'node-fetch';
import { Fetcher, FetcherRequestInit, FetcherResponse } from '@apollo/utils.fetcher';
import { GraphQLError, GraphQLErrorExtensions } from 'graphql';

export class RemoteGraphQLDataSource<
TContext extends Record<string, any> = Record<string, any>,
Expand Down Expand Up @@ -302,28 +298,25 @@ export class RemoteGraphQLDataSource<
}

public async errorFromResponse(response: FetcherResponse) {
const message = `${response.status}: ${response.statusText}`;

let error: ApolloError;
if (response.status === 401) {
error = new AuthenticationError(message);
} else if (response.status === 403) {
error = new ForbiddenError(message);
} else {
error = new ApolloError(message);
}

const body = await this.parseBody(response);

Object.assign(error.extensions, {
const extensions: GraphQLErrorExtensions = {
response: {
url: response.url,
status: response.status,
statusText: response.statusText,
body,
},
});
};

return error;
if (response.status === 401) {
extensions.code = 'UNAUTHENTICATED';
} else if (response.status === 403) {
extensions.code = 'FORBIDDEN';
}

return new GraphQLError(`${response.status}: ${response.statusText}`, {
extensions,
});
}
}
@@ -1,15 +1,10 @@
import {
ApolloError,
AuthenticationError,
ForbiddenError,
} from 'apollo-server-errors';

import { RemoteGraphQLDataSource } from '../RemoteGraphQLDataSource';
import { Response, Headers } from 'node-fetch';
import { GraphQLRequestContext } from 'apollo-server-types';
import { GraphQLDataSourceRequestKind } from '../types';
import { nockBeforeEach, nockAfterEach } from '../../__tests__/nockAssertions';
import nock from 'nock';
import { GraphQLError } from 'graphql';

beforeEach(nockBeforeEach);
afterEach(nockAfterEach);
Expand Down Expand Up @@ -461,15 +456,15 @@ describe('didEncounterError', () => {
context,
});

await expect(result).rejects.toThrow(AuthenticationError);
await expect(result).rejects.toThrow(GraphQLError);
expect(context).toMatchObject({
timingData: [{ time: 1616446845234 }],
});
});
});

describe('error handling', () => {
it('throws an AuthenticationError when the response status is 401', async () => {
it('throws error with code UNAUTHENTICATED when the response status is 401', async () => {
const DataSource = new RemoteGraphQLDataSource({
url: 'https://api.example.com/foo',
});
Expand All @@ -480,7 +475,7 @@ describe('error handling', () => {
...defaultProcessOptions,
request: { query: '{ me { name } }' },
});
await expect(result).rejects.toThrow(AuthenticationError);
await expect(result).rejects.toThrow(GraphQLError);
await expect(result).rejects.toMatchObject({
extensions: {
code: 'UNAUTHENTICATED',
Expand All @@ -492,7 +487,7 @@ describe('error handling', () => {
});
});

it('throws a ForbiddenError when the response status is 403', async () => {
it('throws an error with code FORBIDDEN when the response status is 403', async () => {
const DataSource = new RemoteGraphQLDataSource({
url: 'https://api.example.com/foo',
});
Expand All @@ -503,7 +498,7 @@ describe('error handling', () => {
...defaultProcessOptions,
request: { query: '{ me { name } }' },
});
await expect(result).rejects.toThrow(ForbiddenError);
await expect(result).rejects.toThrow(GraphQLError);
await expect(result).rejects.toMatchObject({
extensions: {
code: 'FORBIDDEN',
Expand All @@ -515,7 +510,7 @@ describe('error handling', () => {
});
});

it('throws an ApolloError when the response status is 500', async () => {
it('throws a GraphQLError when the response status is 500', async () => {
const DataSource = new RemoteGraphQLDataSource({
url: 'https://api.example.com/foo',
});
Expand All @@ -526,7 +521,7 @@ describe('error handling', () => {
...defaultProcessOptions,
request: { query: '{ me { name } }' },
});
await expect(result).rejects.toThrow(ApolloError);
await expect(result).rejects.toThrow(GraphQLError);
await expect(result).rejects.toMatchObject({
extensions: {
response: {
Expand Down Expand Up @@ -560,7 +555,7 @@ describe('error handling', () => {
...defaultProcessOptions,
request: { query: '{ me { name } }' },
});
await expect(result).rejects.toThrow(ApolloError);
await expect(result).rejects.toThrow(GraphQLError);
await expect(result).rejects.toMatchObject({
extensions: {
response: {
Expand Down
2 changes: 0 additions & 2 deletions package-lock.json

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