✨ Treat upstream HTTP and fetch failures as transient, match bare D1 internal error

README.md

@@ -10,6 +10,12 @@ own transient failures the same way.
   their own ("Network connection lost", "storage caused object to be reset",
   "internal error; reference = ...", and so on). A new variant gets added here
   once, then every Worker picks it up on the next version bump.
+- **`isTransientOriginStatus(status)`** / **`isTransientOriginError(error)`**:
+  whether an upstream HTTP status (408, 425, 429, 500 to 530, Cloudflare origin
+  errors included) or a thrown error (a `status` property, a fetch network
+  failure, a `TimeoutError` or `AbortError`, also through `cause` and
+  `AggregateError`) is worth another try. `groupCloudflareErrors` treats these
+  as transient too.
 - **`retryDurableObject(run, options?)`** / **`isRetryableDOError(error)`**:
   the same for Durable Object calls, honouring the runtime's `retryable` and
   `overloaded` flags.

src/d1.ts

@@ -19,6 +19,7 @@ const TRANSIENT_D1 = [
   /d1 db is overloaded/i,
   /d1 db reset/i,
   /internal error; reference/i,
+  /^(d1_error: )?internal error$/i,
 ];
 
 export function isTransientD1Error(error: unknown): boolean {

src/index.ts

@@ -5,6 +5,7 @@ export {
   isTransientQueueError,
   retryQueue,
 } from "./queue.ts";
+export { isTransientOriginError, isTransientOriginStatus } from "./origin.ts";
 export { isRetryableDOError, retryDurableObject } from "./durable-object.ts";
 export {
   TRANSIENT_FINGERPRINT,

src/origin.ts

@@ -0,0 +1,54 @@
+/**
+ * Whether an HTTP status from an upstream is worth another try: request
+ * timeout (408), too early (425), rate limited (429), and every 5xx up to 530,
+ * which takes in Cloudflare's own origin errors (520 to 530).
+ */
+export function isTransientOriginStatus(status: number): boolean {
+  return (
+    status === 408 ||
+    status === 425 ||
+    status === 429 ||
+    (status >= 500 && status <= 530)
+  );
+}
+
+/** undici, workerd and browsers word a failed fetch differently. */
+const FETCH_FAILURE = /fetch failed|failed to fetch|network connection lost/i;
+
+function statusOf(error: object): number | undefined {
+  const { status, statusCode } = error as {
+    status?: unknown;
+    statusCode?: unknown;
+  };
+  if (typeof status === "number") return status;
+  if (typeof statusCode === "number") return statusCode;
+  return undefined;
+}
+
+/**
+ * Whether a thrown error is an upstream blip: it carries a transient `status`
+ * or `statusCode`, or it is a fetch network failure, or a timeout from
+ * `AbortSignal.timeout` (`TimeoutError`) or an abort (`AbortError`). Walks
+ * `cause`, and an AggregateError counts only when it has errors and all of
+ * them are transient.
+ */
+export function isTransientOriginError(error: unknown): boolean {
+  if (error instanceof AggregateError) {
+    return (
+      error.errors.length > 0 && error.errors.every(isTransientOriginError)
+    );
+  }
+  if (typeof error !== "object" || error === null) return false;
+  const status = statusOf(error);
+  if (status !== undefined && isTransientOriginStatus(status)) return true;
+  const { name, message } = error as { name?: unknown; message?: unknown };
+  if (name === "TimeoutError" || name === "AbortError") return true;
+  if (
+    error instanceof TypeError &&
+    typeof message === "string" &&
+    FETCH_FAILURE.test(message)
+  ) {
+    return true;
+  }
+  return isTransientOriginError((error as { cause?: unknown }).cause);
+}

src/sentry.ts

@@ -1,4 +1,5 @@
 import { isTransientD1Error } from "./d1.ts";
+import { isTransientOriginError } from "./origin.ts";
 import { isTransientQueueError } from "./queue.ts";
 
 /**
@@ -34,7 +35,11 @@ function isTransientCloudflareError(error: unknown): boolean {
   if (error instanceof Error && error.cause instanceof Error) {
     return isTransientCloudflareError(error.cause);
   }
-  return isTransientD1Error(error) || isTransientQueueError(error);
+  return (
+    isTransientD1Error(error) ||
+    isTransientQueueError(error) ||
+    isTransientOriginError(error)
+  );
 }
 
 /** Cloudflare's per-occurrence support reference, e.g. `; reference = e_Gz3hrU_03c2...`. */
@@ -47,7 +52,7 @@ const REFERENCE = /;\s*reference\s*=\s*(\S+)/i;
  * GlitchTip groups by the message, so every occurrence of one blip opened its
  * own issue (and the GitHub bridge its own GitHub issue). This strips the
  * reference from the message and keeps it as the `cf_reference` tag, which is
- * what Cloudflare support asks for. A transient error (D1, queue or Durable Object), or an AggregateError whose
+ * what Cloudflare support asks for. A transient error (D1, queue, Durable Object or origin fetch), or an AggregateError whose
  * inner errors all are, that
  * survived the retries also gets one fixed fingerprint, so an outage is one issue per
  * project with an event count, not one issue per failed request.

test/d1.test.ts

@@ -13,13 +13,18 @@ describe("isTransientD1Error", () => {
     "D1_ERROR: internal error; reference = ipvha29hm877ogb0k536c7pn",
     "Durable Object reset because its code was updated.",
     "D1 DB is overloaded. Too many requests queued.",
+    "internal error",
+    "D1_ERROR: internal error",
+    "D1_ERROR: Internal Error",
   ])("retries %s", (message) => {
     expect(isTransientD1Error(new Error(message))).toBe(true);
   });
 
   it.each([
     "D1_ERROR: UNIQUE constraint failed: votes.round_id",
     "D1_ERROR: no such table: nope: SQLITE_ERROR",
+    "validation failed: internal error in handler",
+    "internal error while parsing the row",
   ])("does not retry %s", (message) => {
     expect(isTransientD1Error(new Error(message))).toBe(false);
   });

test/origin.test.ts

@@ -0,0 +1,109 @@
+import { describe, expect, it } from "vitest";
+
+import {
+  isTransientOriginError,
+  isTransientOriginStatus,
+} from "../src/origin.ts";
+import {
+  groupCloudflareErrors,
+  TRANSIENT_FINGERPRINT,
+  type SentryEventLike,
+} from "../src/sentry.ts";
+
+describe("isTransientOriginStatus", () => {
+  it.each([408, 425, 429, 500, 502, 503, 504, 520, 522, 524, 530])(
+    "retries %i",
+    (status) => {
+      expect(isTransientOriginStatus(status)).toBe(true);
+    },
+  );
+  it.each([200, 301, 400, 401, 403, 404, 409, 422, 531, 599])(
+    "does not retry %i",
+    (status) => {
+      expect(isTransientOriginStatus(status)).toBe(false);
+    },
+  );
+});
+
+describe("isTransientOriginError", () => {
+  it("reads status and statusCode", () => {
+    expect(
+      isTransientOriginError(Object.assign(new Error("x"), { status: 503 })),
+    ).toBe(true);
+    expect(
+      isTransientOriginError(
+        Object.assign(new Error("x"), { statusCode: 522 }),
+      ),
+    ).toBe(true);
+    expect(
+      isTransientOriginError(Object.assign(new Error("x"), { status: 404 })),
+    ).toBe(false);
+    expect(
+      isTransientOriginError(Object.assign(new Error("x"), { status: "503" })),
+    ).toBe(false);
+  });
+
+  it("recognises fetch network failures", () => {
+    expect(isTransientOriginError(new TypeError("fetch failed"))).toBe(true);
+    expect(isTransientOriginError(new TypeError("Failed to fetch"))).toBe(true);
+    expect(
+      isTransientOriginError(new TypeError("Network connection lost.")),
+    ).toBe(true);
+    expect(isTransientOriginError(new TypeError("x is not a function"))).toBe(
+      false,
+    );
+    expect(isTransientOriginError(new Error("fetch failed"))).toBe(false);
+  });
+
+  it("recognises timeouts and aborts", () => {
+    expect(
+      isTransientOriginError(new DOMException("timed out", "TimeoutError")),
+    ).toBe(true);
+    expect(
+      isTransientOriginError(new DOMException("aborted", "AbortError")),
+    ).toBe(true);
+    const named = new Error("t");
+    named.name = "TimeoutError";
+    expect(isTransientOriginError(named)).toBe(true);
+  });
+
+  it("walks cause", () => {
+    const error = new Error("sync failed", {
+      cause: new TypeError("fetch failed", { cause: new Error("ECONNRESET") }),
+    });
+    expect(isTransientOriginError(error)).toBe(true);
+    expect(
+      isTransientOriginError(new Error("outer", { cause: new Error("inner") })),
+    ).toBe(false);
+  });
+
+  it("requires every AggregateError member to be transient", () => {
+    const bad = Object.assign(new Error("x"), { status: 502 });
+    expect(isTransientOriginError(new AggregateError([bad, bad], "f"))).toBe(
+      true,
+    );
+    expect(
+      isTransientOriginError(new AggregateError([bad, new Error("y")], "f")),
+    ).toBe(false);
+    expect(isTransientOriginError(new AggregateError([], "f"))).toBe(false);
+  });
+
+  it("ignores non-objects", () => {
+    expect(isTransientOriginError(503)).toBe(false);
+    expect(isTransientOriginError(undefined)).toBe(false);
+    expect(isTransientOriginError(null)).toBe(false);
+  });
+});
+
+describe("groupCloudflareErrors with origin errors", () => {
+  it("fingerprints a transient origin error", () => {
+    const error = Object.assign(new Error("upstream said 503"), {
+      status: 503,
+    });
+    const event = groupCloudflareErrors<SentryEventLike>(
+      { exception: { values: [{ type: "Error", value: error.message }] } },
+      { originalException: error },
+    );
+    expect(event.fingerprint).toEqual(TRANSIENT_FINGERPRINT);
+  });
+});