Skip to content

Commit

Permalink
use Response.json only for non-defer requests
Browse files Browse the repository at this point in the history
  • Loading branch information
@phryneas
phryneas committed Sep 12, 2023
1 parent 3901a46 commit 4802ad2
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 119 deletions.
12 changes: 4 additions & 8 deletions src/link/batch-http/__tests__/batchHttpLink.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1124,12 +1124,9 @@ describe("SharedHttpTest", () => {
},
};

function mockFetch() {
const text = jest.fn(
async () => '{ "data": { "stub": { "id": "foo" } } }'
);
const fetch = jest.fn(async (uri, options) => ({ text }));
return { text, fetch };
function mockFetch(body = '{ "data": { "stub": { "id": "foo" } } }') {
const fetch = jest.fn(async (uri, options) => new Response(body));
return { fetch };
}

it("aborts the request when unsubscribing before the request has completed", () => {
Expand Down Expand Up @@ -1186,9 +1183,8 @@ describe("SharedHttpTest", () => {
});

it("resolving fetch does not cause the AbortController to be aborted", async () => {
const { text, fetch } = mockFetch();
const { fetch } = mockFetch('{ "data": { "hello": "world" } }');
const abortControllers = trackGlobalAbortControllers();
text.mockResolvedValueOnce('{ "data": { "hello": "world" } }');

// (the request is already finished at that point)
const link = createHttpLink({ uri: "data", fetch: fetch as any });
Expand Down
122 changes: 48 additions & 74 deletions src/link/http/__tests__/HttpLink.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -961,16 +961,13 @@ describe("HttpLink", () => {
expect(typeof fetch).toBe("function");

const fetchSpy = jest.spyOn(window, "fetch");
fetchSpy.mockImplementation(() =>
Promise.resolve<Response>({
text() {
return Promise.resolve(
JSON.stringify({
data: { hello: "from spy" },
})
);
},
} as Response)
fetchSpy.mockImplementation(
async () =>
new Response(
JSON.stringify({
data: { hello: "from spy" },
})
)
);

const spyFn = window.fetch;
Expand Down Expand Up @@ -1240,43 +1237,30 @@ describe("HttpLink", () => {
});

describe("Error handling", () => {
let responseBody: any;
const text = jest.fn(() => {
const responseBodyText = "{}";
responseBody = JSON.parse(responseBodyText);
return Promise.resolve(responseBodyText);
const resultEmpty = JSON.stringify({});
const resultWithStringError = "Error! Foo bar";
const resultWithData = JSON.stringify({
data: { stub: { id: 1 } },
errors: [{ message: "dangit" }],
});
const textWithStringError = jest.fn(() => {
const responseBodyText = "Error! Foo bar";
responseBody = responseBodyText;
return Promise.resolve(responseBodyText);
const resultWithErrors = JSON.stringify({
errors: [{ message: "dangit" }],
});
const textWithData = jest.fn(() => {
responseBody = {
data: { stub: { id: 1 } },
errors: [{ message: "dangit" }],
};

return Promise.resolve(JSON.stringify(responseBody));
});

const textWithErrors = jest.fn(() => {
responseBody = {
errors: [{ message: "dangit" }],
};

return Promise.resolve(JSON.stringify(responseBody));
});
const fetch = jest.fn((uri, options) => {
return Promise.resolve({ text });
});
const fetch = jest.fn(async (uri, options) => new Response(resultEmpty));
function mockFetchOnce({
resultBody,
status = 200,
}: Pick<Response, "status"> & { resultBody: string }) {
fetch.mockResolvedValueOnce(new Response(resultBody, { status }));
}
beforeEach(() => {
fetch.mockReset();
});
itAsync("makes it easy to do stuff on a 401", (resolve, reject) => {
const middleware = new ApolloLink((operation, forward) => {
return new Observable((ob) => {
fetch.mockReturnValueOnce(Promise.resolve({ status: 401, text }));
mockFetchOnce({ status: 401, resultBody: resultEmpty });
const op = forward(operation);
const sub = op.subscribe({
next: ob.next.bind(ob),
Expand Down Expand Up @@ -1307,7 +1291,7 @@ describe("HttpLink", () => {
});

itAsync("throws an error if response code is > 300", (resolve, reject) => {
fetch.mockReturnValueOnce(Promise.resolve({ status: 400, text }));
mockFetchOnce({ status: 400, resultBody: resultEmpty });
const link = createHttpLink({ uri: "data", fetch: fetch as any });

execute(link, { query: sampleQuery }).subscribe(
Expand All @@ -1317,16 +1301,17 @@ describe("HttpLink", () => {
makeCallback(resolve, reject, (e: ServerError) => {
expect(e.message).toMatch(/Received status code 400/);
expect(e.statusCode).toBe(400);
expect(e.result).toEqual(responseBody);
expect(e.result).toEqual(JSON.parse(resultEmpty));
})
);
});
itAsync(
"throws an error if response code is > 300 and handles string response body",
(resolve, reject) => {
fetch.mockReturnValueOnce(
Promise.resolve({ status: 302, text: textWithStringError })
);
mockFetchOnce({
status: 302,
resultBody: resultWithStringError as any,
});
const link = createHttpLink({ uri: "data", fetch: fetch as any });
execute(link, { query: sampleQuery }).subscribe(
(result) => {
Expand All @@ -1335,32 +1320,30 @@ describe("HttpLink", () => {
makeCallback(resolve, reject, (e: ServerError) => {
expect(e.message).toMatch(/Received status code 302/);
expect(e.statusCode).toBe(302);
expect(e.result).toEqual(responseBody);
// JSON.parse consumed the buffer, we cannot read it with `.text` anymore
expect(e.result).toEqual(undefined);
})
);
}
);
itAsync(
"throws an error if response code is > 300 and returns data",
(resolve, reject) => {
fetch.mockReturnValueOnce(
Promise.resolve({ status: 400, text: textWithData })
);

mockFetchOnce({ status: 400, resultBody: resultWithData });
const link = createHttpLink({ uri: "data", fetch: fetch as any });

let called = false;

execute(link, { query: sampleQuery }).subscribe(
(result) => {
called = true;
expect(result).toEqual(responseBody);
expect(result).toEqual(JSON.parse(resultWithData));
},
(e) => {
expect(called).toBe(true);
expect(e.message).toMatch(/Received status code 400/);
expect(e.statusCode).toBe(400);
expect(e.result).toEqual(responseBody);
expect(e.result).toEqual(JSON.parse(resultWithData));
resolve();
}
);
Expand All @@ -1369,9 +1352,7 @@ describe("HttpLink", () => {
itAsync(
"throws an error if only errors are returned",
(resolve, reject) => {
fetch.mockReturnValueOnce(
Promise.resolve({ status: 400, text: textWithErrors })
);
mockFetchOnce({ status: 400, resultBody: resultWithErrors });

const link = createHttpLink({ uri: "data", fetch: fetch as any });
execute(link, { query: sampleQuery }).subscribe(
Expand All @@ -1381,7 +1362,7 @@ describe("HttpLink", () => {
(e) => {
expect(e.message).toMatch(/Received status code 400/);
expect(e.statusCode).toBe(400);
expect(e.result).toEqual(responseBody);
expect(e.result).toEqual(JSON.parse(resultWithErrors));
resolve();
}
);
Expand All @@ -1390,8 +1371,10 @@ describe("HttpLink", () => {
itAsync(
"throws an error if empty response from the server ",
(resolve, reject) => {
fetch.mockReturnValueOnce(Promise.resolve({ text }));
text.mockReturnValueOnce(Promise.resolve('{ "body": "boo" }'));
mockFetchOnce({
status: 200,
resultBody: JSON.stringify({ body: "boo" }),
});
const link = createHttpLink({ uri: "data", fetch: fetch as any });

execute(link, { query: sampleQuery }).subscribe(
Expand All @@ -1407,7 +1390,7 @@ describe("HttpLink", () => {
}
);
itAsync("throws if the body can't be stringified", (resolve, reject) => {
fetch.mockReturnValueOnce(Promise.resolve({ data: {}, text }));
// no need to mock fetch, it won't even get that far
const link = createHttpLink({
uri: "data",
fetch: fetch as any,
Expand Down Expand Up @@ -1467,12 +1450,9 @@ describe("HttpLink", () => {
},
};

function mockFetch() {
const text = jest.fn(
async () => '{ "data": { "stub": { "id": "foo" } } }'
);
const fetch = jest.fn(async (uri, options) => ({ text }));
return { text, fetch };
function mockFetch(body = '{ "data": { "stub": { "id": "foo" } } }') {
const fetch = jest.fn(async (uri, options) => new Response(body));
return { fetch };
}

it("aborts the request when unsubscribing before the request has completed", () => {
Expand Down Expand Up @@ -1540,9 +1520,8 @@ describe("HttpLink", () => {
});

it("resolving fetch does not cause the AbortController to be aborted", async () => {
const { text, fetch } = mockFetch();
const { fetch } = mockFetch('{ "data": { "hello": "world" } }');
const abortControllers = trackGlobalAbortControllers();
text.mockResolvedValueOnce('{ "data": { "hello": "world" } }');

// (the request is already finished at that point)
const link = createHttpLink({ uri: "data", fetch: fetch as any });
Expand Down Expand Up @@ -1575,14 +1554,10 @@ describe("HttpLink", () => {
});
});

const body = "{";
const unparsableJson = jest.fn(() => Promise.resolve(body));
itAsync(
"throws a Server error if response is > 300 with unparsable json",
(resolve, reject) => {
fetch.mockReturnValueOnce(
Promise.resolve({ status: 400, text: unparsableJson })
);
mockFetchOnce({ status: 400, resultBody: "{" });
const link = createHttpLink({ uri: "data", fetch: fetch as any });

execute(link, { query: sampleQuery }).subscribe(
Expand All @@ -1604,9 +1579,7 @@ describe("HttpLink", () => {
itAsync(
"throws a ServerParse error if response is 200 with unparsable json",
(resolve, reject) => {
fetch.mockReturnValueOnce(
Promise.resolve({ status: 200, text: unparsableJson })
);
mockFetchOnce({ status: 200, resultBody: "{" });
const link = createHttpLink({ uri: "data", fetch: fetch as any });

execute(link, { query: sampleQuery }).subscribe(
Expand All @@ -1617,7 +1590,8 @@ describe("HttpLink", () => {
expect(e.message).toMatch(/JSON/);
expect(e.statusCode).toBe(200);
expect(e.response).toBeDefined();
expect(e.bodyText).toBe(body);
// bodyText cannot be read a second time - the buffer has been consumed by `Response.json()`
expect(e.bodyText).toBe("");
})
);
}
Expand Down
46 changes: 45 additions & 1 deletion src/link/http/__tests__/parseAndCheckHttpResponse.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ import gql from "graphql-tag";
import fetchMock from "fetch-mock";

import { createOperation } from "../../utils/createOperation";
import { parseAndCheckHttpResponse } from "../parseAndCheckHttpResponse";
import {
parseAndCheckHttpResponse,
parseJsonBody,
} from "../parseAndCheckHttpResponse";
import { itAsync } from "../../../testing";

const query = gql`
Expand Down Expand Up @@ -115,3 +118,44 @@ describe("parseAndCheckResponse", () => {
.catch(reject);
});
});

describe("parseJsonBody", () => {
it("(called with one argument) parses JSON from response without calling JSON.parse", async () => {
const spy = jest.spyOn(JSON, "parse");
try {
const received = { data: new Array(1000).fill({ hello: "world" }) };
const response = new Response(JSON.stringify(received), {
status: 200,
});
const promise = parseJsonBody(response);
await expect(promise).resolves.toEqual(received);

expect(spy).not.toHaveBeenCalled();
} finally {
spy.mockRestore();
}
});

it("(called with two arguments) uses `JSON.parse`", async () => {
const spy = jest.spyOn(JSON, "parse");
const originalResponse = global.Response;
try {
const received = { data: new Array(1000).fill({ hello: "world" }) };
const response = new Response(JSON.stringify(received), {
status: 200,
});
const bodyText = await response.text();

// @ts-expect-error
delete global.Response;

const promise = parseJsonBody(response, bodyText);
await expect(promise).resolves.toEqual(received);

expect(spy).toHaveBeenCalled();
} finally {
spy.mockRestore();
global.Response = originalResponse;
}
});
});
Loading

0 comments on commit 4802ad2

Please sign in to comment.