Fix client interface 4xx retry handling (#1492)

Author: N2D4

packages/stack-shared/src/interface/client-interface.test.ts

@@ -52,6 +52,10 @@ function createKnownErrorResponse(error: InstanceType<typeof KnownErrors[keyof t
   });
 }
 
+function createTextResponse(body: string, options: { status: number, headers?: Record<string, string> }): Response {
+  return new Response(body, options);
+}
+
 function getRequestBody(fetchMock: { mock: { calls: unknown[][] } }): Record<string, unknown> {
   const requestInit = fetchMock.mock.calls[0]?.[1];
   if (requestInit == null || typeof requestInit !== "object" || !("body" in requestInit)) {
@@ -437,6 +441,63 @@ describe("_withFallback", () => {
     expect(log.every(u => urlIndex(urls, u) === 0)).toBe(true);
   });
 
+  it("does not retry or fall back on non-KnownError 4xx responses", async () => {
+    const urls = urlList(3);
+    const log: string[] = [];
+    vi.stubGlobal("fetch", vi.fn(async (input: RequestInfo | URL) => {
+      log.push(input.toString());
+      return createTextResponse("Payments are not set up", { status: 402 });
+    }));
+
+    const iface = createClientInterface({ apiUrls: urls });
+    await expect(sendRequest(iface)).rejects.toMatchObject({ name: "Error" });
+    expect(log.length).toBe(1);
+    expect(urlIndex(urls, log[0])).toBe(0);
+  });
+
+  it("wraps non-KnownError 4xx responses as normal errors", async () => {
+    const response = createTextResponse("Payments are not set up", { status: 402 });
+    vi.stubGlobal("fetch", vi.fn(async () => response));
+
+    const iface = createClientInterface({ apiUrls: urlList(1) });
+    await expect(sendRequest(iface)).rejects.toMatchObject({
+      name: "Error",
+      message: expect.stringContaining("402 Payments are not set up"),
+      cause: response,
+    });
+  });
+
+  it("does not retry non-KnownError 5xx responses on a single URL", async () => {
+    let attempts = 0;
+    vi.stubGlobal("fetch", vi.fn(async () => {
+      attempts++;
+      return createTextResponse("Server unavailable", { status: 503 });
+    }));
+
+    const iface = createClientInterface({ apiUrls: urlList(1) });
+    await expect(sendRequest(iface)).rejects.toThrow("503 Server unavailable");
+    expect(attempts).toBe(1);
+  });
+
+  it("falls back on non-KnownError 5xx responses", async () => {
+    const urls = urlList(3);
+    const log: string[] = [];
+    vi.stubGlobal("fetch", vi.fn(async (input: RequestInfo | URL) => {
+      const url = input.toString();
+      log.push(url);
+      if (urlIndex(urls, url) === 0) {
+        return createTextResponse("Server unavailable", { status: 503 });
+      }
+      return createJsonResponse({ display_name: "test" });
+    }));
+
+    const iface = createClientInterface({ apiUrls: urls });
+    await sendRequest(iface);
+    expect(log.length).toBe(2);
+    expect(urlIndex(urls, log[0])).toBe(0);
+    expect(urlIndex(urls, log[1])).toBe(1);
+  });
+
   it("makes 2 passes × N URLs attempts before throwing", async () => {
     for (const n of [2, 3, 5]) {
       const urls = urlList(n);

packages/stack-shared/src/interface/client-interface.ts

@@ -219,8 +219,8 @@ export class HexclaveClientInterface {
    *   - Sticky URL fails → exit sticky mode, do a full iteration.
    *
    * In both modes, a full iteration tries every URL once per pass for 2
-   * passes before giving up. KnownErrors are never retried (they're
-   * application-level, not network-level).
+   * passes before giving up. KnownErrors and 4xx API responses (except 429)
+   * are never retried (they're application-level, not network-level).
    *
    * Single-URL lists skip all of this and use 5-retry behavior directly.
    */
@@ -243,6 +243,15 @@ export class HexclaveClientInterface {
     return await this._iterateUrls(apiUrls, cb);
   }
 
+  private _shouldSkipFallback(error: unknown) {
+    return error instanceof KnownError || this._isNonRetryableApiResponseError(error);
+  }
+
+  private _isNonRetryableApiResponseError(error: unknown) {
+    const cause = error instanceof Error ? error.cause : undefined;
+    return cause instanceof Response && cause.status >= 400 && cause.status < 500;
+  }
+
   /**
    * Attempts the sticky URL, optionally probing primary first.
    * Returns the result on success, or `undefined` if we should fall through to full iteration.
@@ -260,7 +269,7 @@ export class HexclaveClientInterface {
         this._sticky = null;
         return result;
       } catch (e) {
-        if (e instanceof KnownError) throw e;
+        if (this._shouldSkipFallback(e)) throw e;
         sticky.probeRate = Math.max(sticky.probeRate * 0.5, 0.01);
       }
     }
@@ -269,7 +278,7 @@ export class HexclaveClientInterface {
     try {
       return await cb(apiUrls[sticky.index], { maxAttempts: 1, skipDiagnostics: true });
     } catch (e) {
-      if (e instanceof KnownError) throw e;
+      if (this._shouldSkipFallback(e)) throw e;
       this._sticky = null;
       return undefined;
     }
@@ -294,7 +303,7 @@ export class HexclaveClientInterface {
           }
           return result;
         } catch (e) {
-          if (e instanceof KnownError) throw e;
+          if (this._shouldSkipFallback(e)) throw e;
           lastError = e instanceof Error ? e : new Error(String(e));
         }
       }
@@ -457,7 +466,7 @@ export class HexclaveClientInterface {
 
       if (!response.data.ok) {
         const body = await response.data.text();
-        throw new Error(`Failed to send refresh token request: ${response.status} ${body}`);
+        throw new Error(`Failed to send refresh token request: ${response.status} ${body}`, { cause: response.data });
       }
 
       return response.data;
@@ -777,6 +786,10 @@ export class HexclaveClientInterface {
     } else {
       const error = await res.text();
 
+      // Do not retry, throw error instead of returning one
+      if (res.status >= 400 && res.status < 500) {
+        throw new Error(`Failed to send request to ${url}: ${res.status} ${error}`, { cause: res });
+      }
       const errorObj = new HexclaveAssertionError(`Failed to send request to ${url}: ${res.status} ${error}`, { request: params, res, path });
 
       if (res.status === 508 && error.includes("INFINITE_LOOP_DETECTED")) {