From 21ea8a84a04bf6413b90ff37899734e77a6dfc77 Mon Sep 17 00:00:00 2001
From: Ajay Ghanwat <ghanwat.ajay@workroot.in>
Date: Wed, 5 Aug 2026 13:45:37 +0530
Subject: [PATCH] fix(rpc): bound retryable status range and add
 malformed-response code
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- isRetryableStatus now fails closed for out-of-range values (600+, negative,
  NaN) by bounding the 5xx check on both sides (>= 500 && <= 599), instead of
  an unbounded >= 500 that classified garbage statuses like 1000 as retryable.
- 408 Request Timeout is now retryable, matching the RPC_TRANSPORT doc
  comment (connection, timeout, 5xx) — a timeout surfaced as 408 is no longer
  treated differently from the same timeout surfaced as 504.
- Add RPC_MALFORMED: the callee answered, but not with a ServiceResult (HTML
  error page, truncated body, unexpected shape). Distinct from RPC_TRANSPORT
  since something DID respond; non-retryable via the existing retryableFor,
  no new branch needed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 packages/rpc/src/errors.ts       | 18 +++++++++++++++---
 packages/rpc/test/errors.test.ts | 18 ++++++++++++++++++
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/packages/rpc/src/errors.ts b/packages/rpc/src/errors.ts
index e19d8514..02f7f709 100644
--- a/packages/rpc/src/errors.ts
+++ b/packages/rpc/src/errors.ts
@@ -13,6 +13,12 @@ export const RPC_ERROR_CODES = {
   handler: "RPC_HANDLER",
   /** Identity token missing, malformed, expired, or for another audience. */
   identity: "RPC_IDENTITY",
+  /**
+   * The callee answered, but not with a ServiceResult — a proxy's HTML error
+   * page, a truncated body, an unexpected shape. Distinct from `transport`:
+   * something DID respond, so retrying returns the same thing.
+   */
+  malformed: "RPC_MALFORMED",
 } as const;
 
 export type RpcErrorCode = (typeof RPC_ERROR_CODES)[keyof typeof RPC_ERROR_CODES];
@@ -23,11 +29,17 @@ function retryableFor(code: string): boolean {
 }
 
 /**
- * 5xx and 429 mean "the callee could not answer, try later". A 4xx is the
- * callee saying no — retrying it just repeats the same rejection.
+ * 5xx, 429 and 408 mean "the callee could not answer, try later". Any other
+ * 4xx is the callee saying no — retrying just repeats the same rejection.
+ *
+ * The range is bounded on BOTH sides deliberately: an unbounded `>= 500`
+ * puts a garbage status like 1000 in the retryable bucket, and this function
+ * is the sole gate the client and HTTP transport trust for retry safety.
+ * An out-of-range value must fail closed, i.e. not retryable.
  */
 export function isRetryableStatus(status: number): boolean {
-  return status >= 500 || status === 429;
+  if (status === 408 || status === 429) return true;
+  return status >= 500 && status <= 599;
 }
 
 export function success<T>(value: T): ServiceResult<T> {
diff --git a/packages/rpc/test/errors.test.ts b/packages/rpc/test/errors.test.ts
index 98e2bc0e..61687880 100644
--- a/packages/rpc/test/errors.test.ts
+++ b/packages/rpc/test/errors.test.ts
@@ -23,12 +23,30 @@ describe("rpc errors", () => {
     expect(isRetryableStatus(500)).toBe(true);
     expect(isRetryableStatus(503)).toBe(true);
     expect(isRetryableStatus(429)).toBe(true);
+    expect(isRetryableStatus(599)).toBe(true);
+    expect(isRetryableStatus(408)).toBe(true);
     expect(isRetryableStatus(400)).toBe(false);
     expect(isRetryableStatus(403)).toBe(false);
     expect(isRetryableStatus(404)).toBe(false);
+    expect(isRetryableStatus(409)).toBe(false);
     expect(isRetryableStatus(200)).toBe(false);
   });
 
+  test("an out-of-range status fails closed", () => {
+    for (const status of [600, 1000, 0, -1, Number.NaN]) {
+      expect(isRetryableStatus(status)).toBe(false);
+    }
+  });
+
+  test("a malformed response is never retryable", () => {
+    const f = failure(RPC_ERROR_CODES.malformed, "Unexpected response shape");
+    expect(f.ok).toBe(false);
+    if (!f.ok) {
+      expect(f.code).toBe("RPC_MALFORMED");
+      expect(f.retryable).toBe(false);
+    }
+  });
+
   test("a denial is never retryable", () => {
     const f = failure(RPC_ERROR_CODES.denied, "Forbidden");
     if (!f.ok) expect(f.retryable).toBe(false);