diff --git a/src/fetch.ts b/src/fetch.ts index 10c91583..6c995f82 100644 --- a/src/fetch.ts +++ b/src/fetch.ts @@ -217,7 +217,19 @@ 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 (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; + } } break; } diff --git a/test/index.test.ts b/test/index.test.ts index 5ac20b07..f927392e 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,31 @@ 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 }; + }) + // 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 @@ -250,6 +278,46 @@ 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("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, + 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);