Merge pull request #33 from FoundatioFx/fix/response-deserialization-errors

Treat response deserialization failures as request errors

Author: ejsmith

docs/guide/error-handling.md

@@ -35,19 +35,49 @@ if (response.status === 404) {
 
 ## Prevent All Throwing
 
-Disable throwing entirely:
+Disable throwing for unexpected HTTP status codes:
 
 ```ts
 const response = await client.getJSON("/api/resource", {
   shouldThrowOnUnexpectedStatusCodes: false,
 });
 
-// Always returns response, never throws
+// Returns responses for unexpected HTTP status codes
 if (!response.ok) {
   console.log("Request failed:", response.status);
 }
 ```
 
+Response body consumption, cancellation, and deserialization errors are not HTTP
+status errors and can still reject the request.
+
+## Response Deserialization Errors
+
+JSON helpers throw `FetchClientDeserializationError` when a successful response
+body cannot be read or parsed. The error retains the response, the underlying
+cause, and any response text that was read. Invalid JSON is never returned in
+`response.data`.
+
+```ts
+import { FetchClientDeserializationError } from "@foundatiofx/fetchclient";
+
+try {
+  await client.getJSON("/api/resource");
+} catch (error) {
+  if (error instanceof FetchClientDeserializationError) {
+    console.log(error.response.status);
+    console.log(error.responseText);
+    console.log(error.cause);
+  }
+}
+```
+
+If an `AbortSignal` cancels response body consumption, FetchClient rejects with
+the signal's original abort reason instead of wrapping it as a deserialization
+error. Middleware observes both cancellation and deserialization failures.
+`errorCallback` is invoked for deserialization errors and can explicitly
+suppress them by returning `true`; cancellation bypasses `errorCallback`.
+
 ## Custom Error Callback
 
 Handle errors with custom logic:

mod.ts

@@ -1,7 +1,10 @@
 export { FetchClient } from "./src/FetchClient.ts";
 export type { FetchClientOptions } from "./src/FetchClientOptions.ts";
 export type { FetchClientResponse } from "./src/FetchClientResponse.ts";
-export { FetchClientError } from "./src/FetchClientError.ts";
+export {
+  FetchClientDeserializationError,
+  FetchClientError,
+} from "./src/FetchClientError.ts";
 export { getStatusText } from "./src/HttpStatusText.ts";
 export { ResponsePromise } from "./src/ResponsePromise.ts";
 export { ProblemDetails } from "./src/ProblemDetails.ts";

src/FetchClient.ts

@@ -15,7 +15,10 @@ import { getCurrentProvider } from "./DefaultHelpers.ts";
 import type { FetchClientOptions } from "./FetchClientOptions.ts";
 import { type IObjectEvent, ObjectEvent } from "./ObjectEvent.ts";
 import { ResponsePromise } from "./ResponsePromise.ts";
-import { FetchClientError } from "./FetchClientError.ts";
+import {
+  FetchClientDeserializationError,
+  FetchClientError,
+} from "./FetchClientError.ts";
 import { getStatusText } from "./HttpStatusText.ts";
 
 type Fetch = typeof globalThis.fetch;
@@ -497,7 +500,11 @@ export class FetchClient {
             "application/problem+json",
           )
         ) {
-          ctx.response = await this.getJSONResponse<T>(response, ctx.options);
+          ctx.response = await this.getJSONResponse<T>(
+            response,
+            ctx.options,
+            ctx.request.signal,
+          );
         } else {
           ctx.response = response as FetchClientResponse<T>;
           ctx.response.data = null;
@@ -558,14 +565,23 @@ export class FetchClient {
       meta: {},
     };
 
-    await this.invokeMiddleware(context, middleware);
-
-    this.#counter.decrement();
-    this.#provider.counter.decrement();
-
-    this.validateResponse(context.response, options);
+    try {
+      await this.invokeMiddleware(context, middleware);
+      this.validateResponse(context.response, options);
+      return context.response as FetchClientResponse<T>;
+    } catch (error) {
+      if (
+        error instanceof FetchClientDeserializationError &&
+        options.errorCallback?.(error.response) === true
+      ) {
+        return error.response as FetchClientResponse<T>;
+      }
 
-    return context.response as FetchClientResponse<T>;
+      throw error;
+    } finally {
+      this.#counter.decrement();
+      this.#provider.counter.decrement();
+    }
   }
 
   private async invokeMiddleware(
@@ -607,6 +623,7 @@ export class FetchClient {
   private async getJSONResponse<T>(
     response: Response,
     options: RequestOptions,
+    signal: AbortSignal,
   ): Promise<FetchClientResponse<T>> {
     let data = null;
     let bodyText = "";
@@ -622,12 +639,37 @@ export class FetchClient {
         }
       }
     } catch (error: unknown) {
-      data = new ProblemDetails();
-      data.detail = bodyText;
-      data.title = `Unable to deserialize response data: ${
+      if (signal.aborted) {
+        throw signal.reason;
+      }
+
+      if (error instanceof DOMException && error.name === "AbortError") {
+        throw error;
+      }
+
+      const problem = new ProblemDetails();
+      problem.detail = bodyText;
+      problem.title = `Unable to deserialize response data: ${
         error instanceof Error ? error.message : String(error)
       }`;
-      data.setErrorMessage(data.title);
+      problem.setErrorMessage(problem.title);
+
+      if (response.ok) {
+        const jsonResponse = response as FetchClientResponse<T>;
+        jsonResponse.data = null;
+        jsonResponse.problem = problem;
+        jsonResponse.meta = {
+          links: parseLinkHeader(response.headers.get("Link")) || {},
+        };
+
+        throw new FetchClientDeserializationError(
+          jsonResponse,
+          error,
+          bodyText,
+        );
+      }
+
+      data = problem;
     }
 
     const jsonResponse = response as FetchClientResponse<T>;

src/FetchClientError.ts

@@ -2,7 +2,7 @@ import type { FetchClientResponse } from "./FetchClientResponse.ts";
 import { getStatusText } from "./HttpStatusText.ts";
 
 /**
- * Error wrapper for non-2xx responses.
+ * Error wrapper for request failures with an HTTP response.
  * Exposes the underlying response for compatibility and debugging.
  */
 export class FetchClientError extends Error {
@@ -99,3 +99,24 @@ export class FetchClientError extends Error {
     return this.response.clone();
   }
 }
+
+/**
+ * Error thrown when the body of a successful response cannot be read or
+ * deserialized as JSON.
+ */
+export class FetchClientDeserializationError extends FetchClientError {
+  public override readonly cause: unknown;
+  public readonly responseText: string;
+
+  constructor(
+    response: FetchClientResponse<unknown>,
+    cause: unknown,
+    responseText: string,
+  ) {
+    const detail = cause instanceof Error ? cause.message : String(cause);
+    super(response, `Unable to deserialize response data: ${detail}`);
+    this.name = "FetchClientDeserializationError";
+    this.cause = cause;
+    this.responseText = responseText;
+  }
+}

src/tests/ErrorHandling.test.ts

@@ -3,10 +3,12 @@ import {
   assertEquals,
   assertFalse,
   assertRejects,
+  assertStrictEquals,
   assertStringIncludes,
 } from "@std/assert";
 import {
   FetchClient,
+  FetchClientDeserializationError,
   FetchClientError,
   type FetchClientResponse,
   ProblemDetails,
@@ -260,3 +262,92 @@ Deno.test("problem details are populated on error responses", async () => {
   assert(res.problem.errors.server);
   assertEquals(res.problem.errors.server[0], "Database connection failed");
 });
+
+Deno.test("malformed JSON in a successful response throws a deserialization error", async () => {
+  const provider = new FetchClientProvider();
+  provider.fetch = () =>
+    Promise.resolve(
+      new Response('{"value":', {
+        status: 200,
+        headers: { "Content-Type": "application/json" },
+      }),
+    );
+
+  const client = provider.getFetchClient();
+  let callbackResponse: FetchClientResponse<unknown> | undefined;
+
+  const error = await assertRejects(
+    () =>
+      client.getJSON("https://example.com/malformed", {
+        errorCallback: (response) => {
+          callbackResponse = response;
+          return false;
+        },
+      }),
+    FetchClientDeserializationError,
+  );
+
+  assert(error instanceof FetchClientDeserializationError);
+  assert(error.cause instanceof SyntaxError);
+  assertEquals(error.responseText, '{"value":');
+  assertEquals(error.response.status, 200);
+  assert(error.response.ok);
+  assertEquals(error.response.data, null);
+  assertStringIncludes(
+    error.response.problem.title ?? "",
+    "Unable to deserialize response data",
+  );
+  assertStrictEquals(callbackResponse, error.response);
+  assertEquals(client.requestCount, 0);
+  assertEquals(provider.requestCount, 0);
+});
+
+Deno.test("aborting a successful response body read preserves the abort reason", async () => {
+  const provider = new FetchClientProvider();
+  provider.fetch = (request) => {
+    const signal = request instanceof Request
+      ? request.signal
+      : new Request(request).signal;
+    const body = new ReadableStream<Uint8Array>({
+      start(controller) {
+        controller.enqueue(new TextEncoder().encode('{"value":'));
+        signal.addEventListener(
+          "abort",
+          () => controller.error(signal.reason),
+          { once: true },
+        );
+      },
+    });
+
+    return Promise.resolve(
+      new Response(body, {
+        status: 200,
+        headers: { "Content-Type": "application/json" },
+      }),
+    );
+  };
+
+  const client = provider.getFetchClient();
+  const abortController = new AbortController();
+  const abortReason = new DOMException("Query cancelled", "AbortError");
+  let middlewareSawAbort = false;
+  client.use(async (_context, next) => {
+    try {
+      await next();
+    } catch (error) {
+      middlewareSawAbort = error === abortReason;
+      throw error;
+    }
+  });
+
+  const request = client.getJSON("https://example.com/stream", {
+    signal: abortController.signal,
+  });
+  setTimeout(() => abortController.abort(abortReason), 10);
+
+  const error = await assertRejects(() => request);
+  assertStrictEquals(error, abortReason);
+  assert(middlewareSawAbort);
+  assertEquals(client.requestCount, 0);
+  assertEquals(provider.requestCount, 0);
+});