Skip to content

feat: Support make-fetch-happen fetcher when communicating with services #188

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

2 changes: 1 addition & 1 deletion gateway-js/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

> The changes noted within this `vNEXT` section have not been released yet. New PRs and commits which introduce changes should include an entry in this `vNEXT` section as part of their development. When a release is being prepared, a new header will be (manually) created below and the appropriate changes within that release will be moved into the new section.

- _Nothing yet! Stay tuned!_
- When using a custom `fetcher` on a `RemoteGraphQLDataSource`, use that fetcher's `Request` initialization in order to satisfy and of its own implementation details. This is necessary, for example, when using `make-fetch-happen`. [PR #188](https://github.com/apollographql/federation/pull/188) [Issue #191](https://github.com/apollographql/federation/issues/191)

## v0.20.4

Expand Down
2 changes: 1 addition & 1 deletion gateway-js/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@apollo/gateway",
"version": "0.20.4",
"version": "0.21.0",
Copy link
Member

Choose a reason for hiding this comment

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

While we should have a "major" bump, our bumping apparatus will handle this!

Suggested change
"version": "0.21.0",
"version": "0.20.4",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks a ton for picking this up so soon 👍
Moreover, Thank you for specifically pointing out the issue around retries, I absolutely agree to it. For our case, we just need timeout configurations at the moment, so it should be fine.

One more thing, do we plan to make make-fetch-happen as the default fetcher for downstream services in this release or in the coming ones?

"description": "Apollo Gateway",
"author": "Apollo <[email protected]>",
"main": "dist/index.js",
Expand Down
51 changes: 51 additions & 0 deletions gateway-js/src/__mocks__/make-fetch-happen-fetcher.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/// <reference types="jest" />

import {
fetch,
Response,
BodyInit,
Headers,
HeadersInit
} from 'apollo-server-env';

import fetcher from 'make-fetch-happen';

interface MakeFetchHappenMock extends jest.Mock<typeof fetch> {
mockResponseOnce(data?: any, headers?: HeadersInit, status?: number): this;
mockJSONResponseOnce(data?: object, headers?: HeadersInit): this;
}

const mockMakeFetchHappen = jest.fn<typeof fetch>(fetcher) as MakeFetchHappenMock;

mockMakeFetchHappen.mockResponseOnce = (
data?: BodyInit,
headers?: Headers,
status: number = 200,
) => {
return mockMakeFetchHappen.mockImplementationOnce(async () => {
return new Response(data, {
status,
headers,
});
});
};

mockMakeFetchHappen.mockJSONResponseOnce = (
data = {},
headers?: Headers,
status?: number,
) => {
return mockMakeFetchHappen.mockResponseOnce(
JSON.stringify(data),
Object.assign({ 'Content-Type': 'application/json' }, headers),
status,
);
};

const makeFetchMock = {
makeFetchHappenFetcher: mockMakeFetchHappen,
};

jest.doMock('make-fetch-happen', () => makeFetchMock);

export = makeFetchMock;
15 changes: 5 additions & 10 deletions gateway-js/src/__tests__/gateway/buildService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,7 @@ it('correctly passes the context from ApolloServer to datasources', async () =>
});

expect(fetch).toBeCalledTimes(1);
expect(fetch).toHaveFetched({
url: 'https://api.example.com/foo',
expect(fetch).toHaveFetched('https://api.example.com/foo', {
Comment on lines -81 to +80
Copy link
Member

Choose a reason for hiding this comment

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

I really enjoy that this pattern now matches up with the standard invocation syntax!

body: {
query: `{me{username}}`,
variables: {},
Expand Down Expand Up @@ -125,8 +124,7 @@ it('makes enhanced introspection request using datasource', async () => {

expect(fetch).toBeCalledTimes(1);

expect(fetch).toHaveFetched({
url: 'https://api.example.com/override',
expect(fetch).toHaveFetched('https://api.example.com/override', {
body: {
query: SERVICE_DEFINITION_QUERY,
},
Expand Down Expand Up @@ -171,8 +169,7 @@ it('customizes request on a per-service basis', async () => {

expect(fetch).toBeCalledTimes(3);

expect(fetch).toHaveFetched({
url: 'https://api.example.com/one',
expect(fetch).toHaveFetched('https://api.example.com/one', {
body: {
query: `query __ApolloGetServiceDefinition__ { _service { sdl } }`,
},
Expand All @@ -181,8 +178,7 @@ it('customizes request on a per-service basis', async () => {
},
});

expect(fetch).toHaveFetched({
url: 'https://api.example.com/two',
expect(fetch).toHaveFetched('https://api.example.com/two', {
body: {
query: `query __ApolloGetServiceDefinition__ { _service { sdl } }`,
},
Expand All @@ -191,8 +187,7 @@ it('customizes request on a per-service basis', async () => {
},
});

expect(fetch).toHaveFetched({
url: 'https://api.example.com/three',
expect(fetch).toHaveFetched('https://api.example.com/three', {
body: {
query: `query __ApolloGetServiceDefinition__ { _service { sdl } }`,
},
Expand Down
36 changes: 20 additions & 16 deletions gateway-js/src/__tests__/matchers/toHaveFetched.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Request, RequestInit, Headers } from 'apollo-server-env';
import { RequestInit, Headers } from 'apollo-server-env';

// Make this file a module
// See: https://github.com/microsoft/TypeScript/issues/17736
Expand All @@ -11,36 +11,39 @@ declare global {
}
}

type ExtendedRequest = RequestInit & { url: string };

function prepareHttpRequest(request: ExtendedRequest): Request {
function prepareHttpOptions(requestUrl: string, requestOpts: RequestInit): RequestInit {
const headers = new Headers();
headers.set('Content-Type', 'application/json');
if (request.headers) {
for (let name in request.headers) {
headers.set(name, request.headers[name]);
if (requestOpts.headers) {
for (let name in requestOpts.headers) {
headers.set(name, requestOpts.headers[name]);
}
}

const options: RequestInit = {
const requestHttp = {
method: 'POST',
headers,
body: JSON.stringify(request.body),
url: requestUrl
};

return {
...requestHttp,
body: JSON.stringify(requestOpts.body)
};

return new Request(request.url, options);
}

function toHaveFetched(
this: jest.MatcherUtils,
fetch: jest.SpyInstance,
request: ExtendedRequest,
requestUrl: string,
requestOpts: RequestInit
): { message(): string; pass: boolean } {
const httpRequest = prepareHttpRequest(request);
const httpOptions = prepareHttpOptions(requestUrl, requestOpts);
let pass = false;
let message = () => '';
try {
expect(fetch).toBeCalledWith(httpRequest);
expect(fetch).toBeCalledWith(requestUrl, httpOptions);
pass = true;
} catch (e) {
message = () => e.message;
Expand All @@ -56,13 +59,14 @@ function toHaveFetchedNth(
this: jest.MatcherUtils,
fetch: jest.SpyInstance,
nthCall: number,
request: ExtendedRequest,
requestUrl: string,
requestOpts: RequestInit
): { message(): string; pass: boolean } {
const httpRequest = prepareHttpRequest(request);
const httpOptions = prepareHttpOptions(requestUrl, requestOpts);
let pass = false;
let message = () => '';
try {
expect(fetch).toHaveBeenNthCalledWith(nthCall, httpRequest);
expect(fetch).toHaveBeenNthCalledWith(nthCall, requestUrl, httpOptions);
pass = true;
} catch (e) {
message = () => e.message;
Expand Down
7 changes: 6 additions & 1 deletion gateway-js/src/datasources/RemoteGraphQLDataSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,12 @@ export class RemoteGraphQLDataSource<TContext extends Record<string, any> = Reco

try {
// Use our local `fetcher` to allow for fetch injection
fetchResponse = await this.fetcher(fetchRequest);
// Adding support for custom fetcher such as 'make-fetch-happen' other than default (node fetch);
// for it, we need the Request to be constructed by the fetcher's own Request class
fetchResponse = await this.fetcher(http.url, {
...http,
body: JSON.stringify(requestWithoutHttp)
});

if (!fetchResponse.ok) {
throw await this.errorFromResponse(fetchResponse);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { fetch } from '__mocks__/apollo-server-env';
import { makeFetchHappenFetcher} from '__mocks__/make-fetch-happen-fetcher';

import {
ApolloError,
Expand Down Expand Up @@ -31,9 +32,8 @@ describe('constructing requests', () => {

expect(data).toEqual({ me: 'james' });
expect(fetch).toBeCalledTimes(1);
expect(fetch).toHaveFetched({
url: 'https://api.example.com/foo',
body: { query: '{ me { name } }' },
expect(fetch).toHaveFetched('https://api.example.com/foo', {
body: { query: '{ me { name } }' }
});
});

Expand All @@ -55,8 +55,7 @@ describe('constructing requests', () => {

expect(data).toEqual({ me: 'james' });
expect(fetch).toBeCalledTimes(1);
expect(fetch).toHaveFetched({
url: 'https://api.example.com/foo',
expect(fetch).toHaveFetched('https://api.example.com/foo', {
body: { query: '{ me { name } }', variables: { id: '1' } },
});
});
Expand Down Expand Up @@ -101,8 +100,7 @@ describe('constructing requests', () => {

expect(data).toEqual({ me: 'james' });
expect(fetch).toBeCalledTimes(2);
expect(fetch).toHaveFetchedNth(1, {
url: 'https://api.example.com/foo',
expect(fetch).toHaveFetchedNth(1, 'https://api.example.com/foo', {
body: {
extensions: {
persistedQuery: {
Expand All @@ -112,8 +110,7 @@ describe('constructing requests', () => {
}
},
});
expect(fetch).toHaveFetchedNth(2, {
url: 'https://api.example.com/foo',
expect(fetch).toHaveFetchedNth(2, 'https://api.example.com/foo', {
body: {
query,
extensions: {
Expand Down Expand Up @@ -145,8 +142,7 @@ describe('constructing requests', () => {

expect(data).toEqual({ me: 'james' });
expect(fetch).toBeCalledTimes(2);
expect(fetch).toHaveFetchedNth(1, {
url: 'https://api.example.com/foo',
expect(fetch).toHaveFetchedNth(1, 'https://api.example.com/foo', {
body: {
variables: { id: '1' },
extensions: {
Expand All @@ -157,9 +153,7 @@ describe('constructing requests', () => {
}
},
});

expect(fetch).toHaveFetchedNth(2, {
url: 'https://api.example.com/foo',
expect(fetch).toHaveFetchedNth(2, 'https://api.example.com/foo', {
body: {
query,
variables: { id: '1' },
Expand Down Expand Up @@ -190,9 +184,8 @@ describe('constructing requests', () => {

expect(data).toEqual({ me: 'james' });
expect(fetch).toBeCalledTimes(1);
expect(fetch).toHaveFetched({
url: 'https://api.example.com/foo',
body: {
expect(fetch).toHaveFetched('https://api.example.com/foo', {
body: {
extensions: {
persistedQuery: {
version: 1,
Expand Down Expand Up @@ -221,8 +214,7 @@ describe('constructing requests', () => {

expect(data).toEqual({ me: 'james' });
expect(fetch).toBeCalledTimes(1);
expect(fetch).toHaveFetched({
url: 'https://api.example.com/foo',
expect(fetch).toHaveFetched('https://api.example.com/foo', {
body: {
variables: { id: '1' },
extensions: {
Expand Down Expand Up @@ -259,6 +251,25 @@ describe('fetcher', () => {

});

it('supports a custom fetcher, like `make-fetch-happen`', async () => {
const injectedFetch =
makeFetchHappenFetcher.mockJSONResponseOnce({ data: { me: 'james' } });
const DataSource = new RemoteGraphQLDataSource({
url: 'https://api.example.com/foo',
fetcher: injectedFetch,
});

const { data } = await DataSource.process({
request: {
query: '{ me { name } }',
variables: { id: '1' },
},
context: {},
});

expect(injectedFetch).toHaveBeenCalled();
expect(data).toEqual({ me: 'james' });
})
});

describe('willSendRequest', () => {
Expand All @@ -281,8 +292,7 @@ describe('willSendRequest', () => {
});

expect(data).toEqual({ me: 'james' });
expect(fetch).toHaveFetched({
url: 'https://api.example.com/foo',
expect(fetch).toHaveFetched('https://api.example.com/foo', {
body: {
query: '{ me { name } }',
variables: JSON.stringify({ id: '1' }),
Expand All @@ -309,8 +319,7 @@ describe('willSendRequest', () => {
});

expect(data).toEqual({ me: 'james' });
expect(fetch).toHaveFetched({
url: 'https://api.example.com/foo',
expect(fetch).toHaveFetched('https://api.example.com/foo', {
body: {
query: '{ me { name } }',
variables: { id: '1' },
Expand Down