From 0459be37d02a1bffcf884ceb84d4d4065c60ebcd Mon Sep 17 00:00:00 2001 From: djordy Date: Tue, 5 Nov 2024 13:01:04 +0100 Subject: [PATCH 1/5] refactor: re-use logic to parse body --- packages/openapi-fetch/src/index.js | 31 +++++++++++++++++------------ 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/packages/openapi-fetch/src/index.js b/packages/openapi-fetch/src/index.js index 1b8502c6f..343306d21 100644 --- a/packages/openapi-fetch/src/index.js +++ b/packages/openapi-fetch/src/index.js @@ -150,28 +150,33 @@ export default function createClient(clientOptions) { } } + const resultKey = response.ok ? "data" : "error"; + // handle empty content if (response.status === 204 || response.headers.get("Content-Length") === "0") { - return response.ok ? { data: undefined, response } : { error: undefined, response }; + return { [resultKey]: undefined, response }; } - // parse response (falling back to .text() when necessary) - if (response.ok) { - // if "stream", skip parsing entirely - if (parseAs === "stream") { - return { data: response.body, response }; - } - return { data: await response[parseAs](), response }; + if (parseAs === "stream") { + return { [resultKey]: response.body, response }; } - // handle errors - let error = await response.text(); + // parse response (falling back to .text() when necessary) try { - error = JSON.parse(error); // attempt to parse as JSON + const fallbackResponseClone = response.clone(); + + return { [resultKey]: await fallbackResponseClone[parseAs](), response }; } catch { - // noop + // handle errors + let data = await response.text(); + try { + data = JSON.parse(data); // attempt to parse as JSON + } catch { + // noop + } + + return { [resultKey]: data, response }; } - return { error, response }; } return { From 55c4a92a5240b561e7e12b44310512cb20fe4442 Mon Sep 17 00:00:00 2001 From: djordy Date: Tue, 5 Nov 2024 13:41:29 +0100 Subject: [PATCH 2/5] fix: future proof empty body --- packages/openapi-fetch/src/index.js | 13 ++++-- .../test/common/response.test.ts | 8 ++++ .../test/common/schemas/common.d.ts | 40 +++++++++++++++++++ .../test/common/schemas/common.yaml | 7 ++++ 4 files changed, 65 insertions(+), 3 deletions(-) diff --git a/packages/openapi-fetch/src/index.js b/packages/openapi-fetch/src/index.js index 343306d21..04840f191 100644 --- a/packages/openapi-fetch/src/index.js +++ b/packages/openapi-fetch/src/index.js @@ -152,8 +152,12 @@ export default function createClient(clientOptions) { const resultKey = response.ok ? "data" : "error"; - // handle empty content - if (response.status === 204 || response.headers.get("Content-Length") === "0") { + /** + * handle empty content + * NOTE: Current browsers don't actually conform to the spec requirement to set the body property to null for responses with no body + * @see https://developer.mozilla.org/en-US/docs/Web/API/Response/body + */ + if (response.body === null || response.headers.get("Content-Length") === "0") { return { [resultKey]: undefined, response }; } @@ -172,7 +176,10 @@ export default function createClient(clientOptions) { try { data = JSON.parse(data); // attempt to parse as JSON } catch { - // noop + // Handle empty content + if (data === "") { + data = undefined; + } } return { [resultKey]: data, response }; diff --git a/packages/openapi-fetch/test/common/response.test.ts b/packages/openapi-fetch/test/common/response.test.ts index 7a3aacf39..193abeb84 100644 --- a/packages/openapi-fetch/test/common/response.test.ts +++ b/packages/openapi-fetch/test/common/response.test.ts @@ -123,6 +123,14 @@ describe("response", () => { assertType(error); } }); + + test("fallback on empty response", async () => { + const client = createObservedClient({}, async () => new Response(undefined, { status: 200 })); + + const { data, error } = await client.GET("/error-empty-response"); + expect(data).toBe(undefined); + expect(error).toBe(undefined); + }); }); describe("response object", () => { diff --git a/packages/openapi-fetch/test/common/schemas/common.d.ts b/packages/openapi-fetch/test/common/schemas/common.d.ts index b684c117e..13e7b96d9 100644 --- a/packages/openapi-fetch/test/common/schemas/common.d.ts +++ b/packages/openapi-fetch/test/common/schemas/common.d.ts @@ -190,6 +190,46 @@ export interface paths { patch?: never; trace?: never; }; + "/error-empty-response": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + /** @description No content */ + 204: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + }; + }; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/error-default": { parameters: { query?: never; diff --git a/packages/openapi-fetch/test/common/schemas/common.yaml b/packages/openapi-fetch/test/common/schemas/common.yaml index 72e1ea6e2..7f3876d77 100644 --- a/packages/openapi-fetch/test/common/schemas/common.yaml +++ b/packages/openapi-fetch/test/common/schemas/common.yaml @@ -79,6 +79,13 @@ paths: application/json: schema: $ref: "#/components/schemas/Error" + /error-empty-response: + get: + responses: + 200: + description: OK + 204: + description: No content /error-default: get: responses: From 5488d5d859a1167049046f36cd881e8ce898c1be Mon Sep 17 00:00:00 2001 From: djordy Date: Tue, 5 Nov 2024 13:41:47 +0100 Subject: [PATCH 3/5] ts-fix: parse error as --- packages/openapi-fetch/src/index.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/openapi-fetch/src/index.d.ts b/packages/openapi-fetch/src/index.d.ts index a7b26a1be..84dea3c6c 100644 --- a/packages/openapi-fetch/src/index.d.ts +++ b/packages/openapi-fetch/src/index.d.ts @@ -106,7 +106,7 @@ export type FetchResponse, Options, Media } | { data?: never; - error: ErrorResponse, Media>; + error: ParseAsResponse, Media>, Options>; response: Response; }; From 23007d283b16cca56d0e49372206889e93301a7b Mon Sep 17 00:00:00 2001 From: djordy Date: Tue, 5 Nov 2024 13:48:06 +0100 Subject: [PATCH 4/5] tests --- .../test/common/response.test.ts | 72 ++++++++++++++++++- .../never-response/never-response.test.ts | 28 ++++++++ 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/packages/openapi-fetch/test/common/response.test.ts b/packages/openapi-fetch/test/common/response.test.ts index 193abeb84..cb674cc32 100644 --- a/packages/openapi-fetch/test/common/response.test.ts +++ b/packages/openapi-fetch/test/common/response.test.ts @@ -124,13 +124,21 @@ describe("response", () => { } }); - test("fallback on empty response", async () => { + test("fallback on null response", async () => { const client = createObservedClient({}, async () => new Response(undefined, { status: 200 })); const { data, error } = await client.GET("/error-empty-response"); expect(data).toBe(undefined); expect(error).toBe(undefined); }); + + test("fallback on empty body steam", async () => { + const client = createObservedClient({}, async () => new Response("", { status: 200 })); + + const { data, error } = await client.GET("/error-empty-response"); + expect(data).toBe(undefined); + expect(error).toBe(undefined); + }); }); describe("response object", () => { @@ -143,7 +151,7 @@ describe("response", () => { }); }); - describe("parseAs", () => { + describe("data parseAs", () => { const client = createObservedClient({}, async () => Response.json({})); test("text", async () => { @@ -200,4 +208,64 @@ describe("response", () => { } }); }); + + describe("error parseAs", () => { + const client = createObservedClient({}, async () => Response.json({}, { status: 500 })); + + test("text", async () => { + const { data, error } = (await client.GET("/resources", { + parseAs: "text", + })) satisfies { error?: string }; + if (data) { + throw new Error("parseAs text: error"); + } + expect(error).toBe("{}"); + }); + + test("arrayBuffer", async () => { + const { data, error } = (await client.GET("/resources", { + parseAs: "arrayBuffer", + })) satisfies { error?: ArrayBuffer }; + if (data) { + throw new Error("parseAs arrayBuffer: error"); + } + expect(error.byteLength).toBe("{}".length); + }); + + test("blob", async () => { + const { data, error } = (await client.GET("/resources", { + parseAs: "blob", + })) satisfies { error?: Blob }; + if (data) { + throw new Error("parseAs blob: error"); + } + expect(error.constructor.name).toBe("Blob"); + }); + + test("stream", async () => { + const { error } = (await client.GET("/resources", { + parseAs: "stream", + })) satisfies { error?: ReadableStream | null }; + if (!error) { + throw new Error("parseAs stream: error"); + } + + expect(error).toBeInstanceOf(ReadableStream); + const reader = error.getReader(); + const result = await reader.read(); + expect(result.value?.length).toBe(2); + }); + + test("use the selected content", async () => { + const client = createObservedClient({}, async () => + Response.json({ bar: "bar" }, { status: 500 }), + ); + const { error } = await client.GET("/media-multiple", { + headers: { Accept: "application/ld+json" }, + }); + if (error) { + assertType<{ bar: string }>(error); + } + }); + }); }); diff --git a/packages/openapi-fetch/test/never-response/never-response.test.ts b/packages/openapi-fetch/test/never-response/never-response.test.ts index 43cdb9a4b..202ddb363 100644 --- a/packages/openapi-fetch/test/never-response/never-response.test.ts +++ b/packages/openapi-fetch/test/never-response/never-response.test.ts @@ -140,4 +140,32 @@ describe("GET", () => { expect(data).toBeUndefined(); expect(error).toBe("Unauthorized"); }); + + describe("handles error as", () => { + test("text", async () => { + const client = createObservedClient({}, async () => new Response("Unauthorized", { status: 401 })); + + const { data, error } = await client.GET("/posts", { parseAs: "text" }); + + expect(data).toBeUndefined(); + expect(error).toBe("Unauthorized"); + }); + + test("stream", async () => { + const client = createObservedClient({}, async () => new Response("Unauthorized", { status: 401 })); + + const { data, error } = (await client.GET("/posts", { parseAs: "stream" })) satisfies { + error?: ReadableStream | null; + }; + if (!error) { + throw new Error("parseAs stream: error"); + } + + expect(data).toBeUndefined(); + expect(error).toBeInstanceOf(ReadableStream); + const reader = error.getReader(); + const result = await reader.read(); + expect(result.value?.length).toBe(12); + }); + }); }); From 0345a28c925757350491e442336e2fcb0d2f2798 Mon Sep 17 00:00:00 2001 From: djordy Date: Tue, 5 Nov 2024 13:54:31 +0100 Subject: [PATCH 5/5] changeset --- .changeset/great-zoos-live.md | 5 +++++ .changeset/thirty-singers-join.md | 5 +++++ 2 files changed, 10 insertions(+) create mode 100644 .changeset/great-zoos-live.md create mode 100644 .changeset/thirty-singers-join.md diff --git a/.changeset/great-zoos-live.md b/.changeset/great-zoos-live.md new file mode 100644 index 000000000..6d9286f71 --- /dev/null +++ b/.changeset/great-zoos-live.md @@ -0,0 +1,5 @@ +--- +"openapi-fetch": patch +--- + +fixed `parseAs` behaviour being different for error responses diff --git a/.changeset/thirty-singers-join.md b/.changeset/thirty-singers-join.md new file mode 100644 index 000000000..2270bc54e --- /dev/null +++ b/.changeset/thirty-singers-join.md @@ -0,0 +1,5 @@ +--- +"openapi-fetch": patch +--- + +fixed empty responses body causing error `Unexpected end of JSON input`