From 8a36bee43e52a19db9fb1ec3cb424385cfcd1373 Mon Sep 17 00:00:00 2001 From: Yarchik Date: Wed, 1 Jul 2026 22:36:44 +0100 Subject: [PATCH 1/4] fix: don't throw SyntaxError on non-JSON error response bodies When a response has no Content-Type header, detectResponseType defaults to "json", so the response body is run through native JSON.parse. That parse ran before the status check and outside any try/catch, so a plain-text 4xx/5xx body (common from nginx/HAProxy/CDN error pages) threw a bare SyntaxError instead of a FetchError. That error carried no status/data, silently skipped retry for retryable codes such as 503, and bypassed ignoreResponseError/onResponseError. Wrap the parse and fall back to the raw text when it is not valid JSON. Valid JSON still parses to an object; error and non-JSON bodies resolve to a FetchError and go through the normal retry and error-handling paths. This restores the non-throwing behavior from before the switch to native JSON.parse, without re-adding the destr dependency. --- src/fetch.ts | 9 +++++++- test/index.test.ts | 57 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/src/fetch.ts b/src/fetch.ts index 10c91583..97e3438c 100644 --- a/src/fetch.ts +++ b/src/fetch.ts @@ -217,7 +217,14 @@ export function createFetch(globalOptions: CreateFetchOptions = {}): $Fetch { const data = await context.response.text(); if (data) { const parseFunction = context.options.parseResponse || JSON.parse; - context.response._data = parseFunction(data); + try { + context.response._data = parseFunction(data); + } catch { + // Body is not valid JSON: keep the raw text so that error + // responses still resolve to a FetchError and non-JSON bodies + // don't throw a SyntaxError that bypasses retry/ignoreResponseError. + context.response._data = data; + } } break; } diff --git a/test/index.test.ts b/test/index.test.ts index 5ac20b07..e6b8d328 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -18,6 +18,9 @@ describe("ofetch", () => { const fetch = vi.spyOn(globalThis, "fetch"); + // Counter for the flaky route used by the non-JSON error retry test. + let flaky503Attempts = 0; + beforeAll(async () => { const app = new H3({ debug: true }) // .use(async (event) => { @@ -52,6 +55,24 @@ describe("ofetch", () => { () => new HTTPError({ status: 403, statusMessage: "Forbidden" }) ) .all("/408", () => new HTTPError({ status: 408 })) + // Plain-text 503 with no Content-Type header, like a bare error page + // emitted by nginx/HAProxy/a CDN in front of the origin. + .all("/text-503", () => { + const response = new Response("Service Unavailable", { status: 503 }); + response.headers.delete("content-type"); + return response; + }) + // Same non-JSON 503 on the first attempt, then a JSON 200, to verify the + // retry path still runs for a retryable status with a non-JSON body. + .all("/flaky-503", () => { + flaky503Attempts++; + if (flaky503Attempts < 2) { + const response = new Response("Service Unavailable", { status: 503 }); + response.headers.delete("content-type"); + return response; + } + return { ok: true }; + }) .all( "/204", () => null // eslint-disable-line unicorn/no-null @@ -250,6 +271,42 @@ describe("ofetch", () => { expect(res?.message).to.eq("Forbidden"); }); + it("non-JSON error body without content-type resolves to a FetchError", async () => { + // A plain-text 4xx/5xx with no Content-Type must not throw a bare + // SyntaxError from JSON.parse; it must surface as a FetchError carrying + // the status and the raw text body. + const error = await $fetch(getURL("text-503"), { retry: 0 }).catch( + (error_: any) => error_ + ); + expect(error.constructor.name).to.equal("FetchError"); + expect(error.status).to.equal(503); + expect(error.data).to.equal("Service Unavailable"); + }); + + it("retries a retryable status whose body is non-JSON text", async () => { + flaky503Attempts = 0; + const result = await $fetch(getURL("flaky-503"), { + retry: 3, + retryDelay: 0, + }); + expect(result).to.deep.equal({ ok: true }); + expect(flaky503Attempts).to.equal(2); + }); + + it("non-JSON error body is swallowed by ignoreResponseError", async () => { + const res = await $fetch(getURL("text-503"), { + ignoreResponseError: true, + retry: 0, + }); + expect(res).to.equal("Service Unavailable"); + }); + + it("valid JSON without content-type still parses to an object", async () => { + flaky503Attempts = 1; // force /flaky-503 to answer 200 JSON immediately + const res = await $fetch(getURL("flaky-503"), { retry: 0 }); + expect(res).to.deep.equal({ ok: true }); + }); + it("204 no content", async () => { const res = await $fetch(getURL("204")); expect(res).toBe(undefined); From aec74d59e19f4cfadcdcbbf14ccf3ba02604bebb Mon Sep 17 00:00:00 2001 From: Yarchik Date: Thu, 2 Jul 2026 16:07:39 +0100 Subject: [PATCH 2/4] fix: only swallow a JSON parse error on error responses --- src/fetch.ts | 13 +++++++++---- test/index.test.ts | 11 +++++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/fetch.ts b/src/fetch.ts index 97e3438c..6c995f82 100644 --- a/src/fetch.ts +++ b/src/fetch.ts @@ -219,10 +219,15 @@ export function createFetch(globalOptions: CreateFetchOptions = {}): $Fetch { const parseFunction = context.options.parseResponse || JSON.parse; try { context.response._data = parseFunction(data); - } catch { - // Body is not valid JSON: keep the raw text so that error - // responses still resolve to a FetchError and non-JSON bodies - // don't throw a SyntaxError that bypasses retry/ignoreResponseError. + } catch (error) { + // A successful response with a malformed JSON body is a real + // problem, so keep throwing there. For an error response the body + // is often not JSON (an HTML error page, plain text), and throwing + // a SyntaxError would bypass retry and ignoreResponseError, so keep + // the raw text and let it resolve to a FetchError instead. + if (context.response.ok) { + throw error; + } context.response._data = data; } } diff --git a/test/index.test.ts b/test/index.test.ts index e6b8d328..f927392e 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -73,6 +73,13 @@ describe("ofetch", () => { } return { ok: true }; }) + // A successful (2xx) response whose JSON body is malformed. A parse + // failure here is a real error and must still surface. + .all("/bad-json-200", () => { + const response = new Response("{ not: valid", { status: 200 }); + response.headers.set("content-type", "application/json"); + return response; + }) .all( "/204", () => null // eslint-disable-line unicorn/no-null @@ -293,6 +300,10 @@ describe("ofetch", () => { expect(flaky503Attempts).to.equal(2); }); + it("throws when a successful response has a malformed JSON body", async () => { + await expect($fetch(getURL("bad-json-200"))).rejects.toThrow(); + }); + it("non-JSON error body is swallowed by ignoreResponseError", async () => { const res = await $fetch(getURL("text-503"), { ignoreResponseError: true, From 7b7769683510f38be7cb42793855d0e71f1c1f12 Mon Sep 17 00:00:00 2001 From: Yarchik Date: Tue, 20 Oct 2026 22:56:28 +0200 Subject: [PATCH 3/4] test: serve the JSON 200 without a content-type header --- test/index.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/index.test.ts b/test/index.test.ts index f927392e..101f7ab0 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -71,7 +71,9 @@ describe("ofetch", () => { response.headers.delete("content-type"); return response; } - return { ok: true }; + const response = new Response(JSON.stringify({ ok: true })); + response.headers.delete("content-type"); + return response; }) // A successful (2xx) response whose JSON body is malformed. A parse // failure here is a real error and must still surface. From b2b88ba6fab19e50a3d627efe3b7e010e49d9202 Mon Sep 17 00:00:00 2001 From: Yarchik Date: Tue, 20 Oct 2026 22:56:28 +0200 Subject: [PATCH 4/4] chore: remove explanatory comments --- src/fetch.ts | 5 ----- test/index.test.ts | 12 +----------- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/src/fetch.ts b/src/fetch.ts index 6c995f82..e5356511 100644 --- a/src/fetch.ts +++ b/src/fetch.ts @@ -220,11 +220,6 @@ export function createFetch(globalOptions: CreateFetchOptions = {}): $Fetch { try { context.response._data = parseFunction(data); } catch (error) { - // A successful response with a malformed JSON body is a real - // problem, so keep throwing there. For an error response the body - // is often not JSON (an HTML error page, plain text), and throwing - // a SyntaxError would bypass retry and ignoreResponseError, so keep - // the raw text and let it resolve to a FetchError instead. if (context.response.ok) { throw error; } diff --git a/test/index.test.ts b/test/index.test.ts index 101f7ab0..8059e325 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -18,7 +18,6 @@ describe("ofetch", () => { const fetch = vi.spyOn(globalThis, "fetch"); - // Counter for the flaky route used by the non-JSON error retry test. let flaky503Attempts = 0; beforeAll(async () => { @@ -55,15 +54,11 @@ describe("ofetch", () => { () => new HTTPError({ status: 403, statusMessage: "Forbidden" }) ) .all("/408", () => new HTTPError({ status: 408 })) - // Plain-text 503 with no Content-Type header, like a bare error page - // emitted by nginx/HAProxy/a CDN in front of the origin. .all("/text-503", () => { const response = new Response("Service Unavailable", { status: 503 }); response.headers.delete("content-type"); return response; }) - // Same non-JSON 503 on the first attempt, then a JSON 200, to verify the - // retry path still runs for a retryable status with a non-JSON body. .all("/flaky-503", () => { flaky503Attempts++; if (flaky503Attempts < 2) { @@ -75,8 +70,6 @@ describe("ofetch", () => { response.headers.delete("content-type"); return response; }) - // A successful (2xx) response whose JSON body is malformed. A parse - // failure here is a real error and must still surface. .all("/bad-json-200", () => { const response = new Response("{ not: valid", { status: 200 }); response.headers.set("content-type", "application/json"); @@ -281,9 +274,6 @@ describe("ofetch", () => { }); it("non-JSON error body without content-type resolves to a FetchError", async () => { - // A plain-text 4xx/5xx with no Content-Type must not throw a bare - // SyntaxError from JSON.parse; it must surface as a FetchError carrying - // the status and the raw text body. const error = await $fetch(getURL("text-503"), { retry: 0 }).catch( (error_: any) => error_ ); @@ -315,7 +305,7 @@ describe("ofetch", () => { }); it("valid JSON without content-type still parses to an object", async () => { - flaky503Attempts = 1; // force /flaky-503 to answer 200 JSON immediately + flaky503Attempts = 1; const res = await $fetch(getURL("flaky-503"), { retry: 0 }); expect(res).to.deep.equal({ ok: true }); });