From 576941c4229bfa018c84bc0b341a0dc28710df19 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 14 Jul 2026 13:30:41 +0000 Subject: [PATCH 1/3] fix(run-engine): don't mislabel DB errors as UnclassifiableWaitpointId in completeWaitpoint forWaitpointCompletion resolves the owning store by probing the database, so a transient DB/infra error surfaced from that call was being caught and rethrown as UnclassifiableWaitpointId with a misleading "length matches neither cuid nor run-ops id" message, losing the original error's type, retryability, and error grouping. Narrow the catch so only a genuine id-classification failure (UnclassifiableRunId) becomes UnclassifiableWaitpointId; every other error (including DB connectivity failures) is rethrown unchanged. --- .../src/engine/systems/waitpointSystem.ts | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/internal-packages/run-engine/src/engine/systems/waitpointSystem.ts b/internal-packages/run-engine/src/engine/systems/waitpointSystem.ts index a0d5b73349..b1143366aa 100644 --- a/internal-packages/run-engine/src/engine/systems/waitpointSystem.ts +++ b/internal-packages/run-engine/src/engine/systems/waitpointSystem.ts @@ -1,5 +1,5 @@ import { timeoutError, tryCatch } from "@trigger.dev/core/v3"; -import { WaitpointId } from "@trigger.dev/core/v3/isomorphic"; +import { UnclassifiableRunId, WaitpointId } from "@trigger.dev/core/v3/isomorphic"; import type { PrismaClientOrTransaction, TaskRun, @@ -91,11 +91,24 @@ export class WaitpointSystem { try { store = await this.$.runStore.forWaitpointCompletion(id, { routeKind: "MANUAL" }); } catch (error) { - this.$.logger.error("completeWaitpoint: unclassifiable waitpointId", { + // Only a genuine id-classification failure should become UnclassifiableWaitpointId. + // forWaitpointCompletion also probes the DB to resolve the owning store, so a transient + // database/infra error (e.g. can't reach the database) can surface here too. Those MUST + // bubble up unchanged so they keep their original type, retryability, and error grouping + // instead of being mislabelled as an unclassifiable id. + if (error instanceof UnclassifiableRunId) { + this.$.logger.error("completeWaitpoint: unclassifiable waitpointId", { + waitpointId: id, + error, + }); + throw new UnclassifiableWaitpointId(id, { cause: error }); + } + + this.$.logger.error("completeWaitpoint: error resolving waitpoint store", { waitpointId: id, error, }); - throw new UnclassifiableWaitpointId(id, { cause: error }); + throw error; } // 1. Complete the Waitpoint (if not completed) From a743d33314901d7a9339215617e61762150a135e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 14 Jul 2026 13:45:40 +0000 Subject: [PATCH 2/3] test(run-engine): cover completeWaitpoint error passthrough vs classification wrapping Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PVbKhNnh8A5tWDGKq9GggV --- ...mpleteWaitpointErrorClassification.test.ts | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 internal-packages/run-engine/src/engine/tests/completeWaitpointErrorClassification.test.ts diff --git a/internal-packages/run-engine/src/engine/tests/completeWaitpointErrorClassification.test.ts b/internal-packages/run-engine/src/engine/tests/completeWaitpointErrorClassification.test.ts new file mode 100644 index 0000000000..d84b380a37 --- /dev/null +++ b/internal-packages/run-engine/src/engine/tests/completeWaitpointErrorClassification.test.ts @@ -0,0 +1,68 @@ +// completeWaitpoint's store-selection guard must only turn a genuine id-classification +// failure into UnclassifiableWaitpointId. forWaitpointCompletion also probes the DB to +// resolve the owning store, so a transient database/infra error can surface from the same +// call — and those must bubble up UNCHANGED (keeping their original type, retryability, and +// error grouping) rather than being mislabelled as an unclassifiable id. +// +// This is a hermetic unit test: the error is thrown on the very first line of +// completeWaitpoint (runStore.forWaitpointCompletion), before any snapshot/enqueue work, +// so we can drive it with a minimal SystemResources and a fake runStore — no DB, no Redis. +import { UnclassifiableRunId } from "@trigger.dev/core/v3/isomorphic"; +import { expect } from "vitest"; +import { UnclassifiableWaitpointId } from "../errors.js"; +import type { SystemResources } from "../systems/systems.js"; +import { WaitpointSystem } from "../systems/waitpointSystem.js"; + +function createWaitpointSystem(forWaitpointCompletion: () => Promise) { + const runStore = { forWaitpointCompletion }; + + const resources = { + runStore, + logger: { + error: vi.fn(), + warn: vi.fn(), + info: vi.fn(), + debug: vi.fn(), + }, + } as unknown as SystemResources; + + return new WaitpointSystem({ + resources, + // Never reached on the store-resolution error path. + executionSnapshotSystem: {} as any, + enqueueSystem: {} as any, + }); +} + +describe("completeWaitpoint store-resolution error classification", () => { + it("rethrows a transient database error unchanged (never wraps it as UnclassifiableWaitpointId)", async () => { + const dbError = new Error("Can't reach database server at db:5432"); + const waitpointSystem = createWaitpointSystem(() => Promise.reject(dbError)); + + // The original error bubbles up as-is... + await expect(waitpointSystem.completeWaitpoint({ id: "waitpoint_transient" })).rejects.toBe( + dbError + ); + // ...and is NOT relabelled as a classification failure. + await expect( + waitpointSystem.completeWaitpoint({ id: "waitpoint_transient" }) + ).rejects.not.toBeInstanceOf(UnclassifiableWaitpointId); + }); + + it("wraps a genuine UnclassifiableRunId as UnclassifiableWaitpointId with the original as cause", async () => { + const waitpointId = "waitpoint_unclassifiable"; + const classificationError = new UnclassifiableRunId(waitpointId); + const waitpointSystem = createWaitpointSystem(() => Promise.reject(classificationError)); + + await expect( + waitpointSystem.completeWaitpoint({ id: waitpointId }) + ).rejects.toBeInstanceOf(UnclassifiableWaitpointId); + + const caught = (await waitpointSystem + .completeWaitpoint({ id: waitpointId }) + .catch((error: unknown) => error)) as UnclassifiableWaitpointId; + expect(caught).toBeInstanceOf(UnclassifiableWaitpointId); + expect(caught.waitpointId).toBe(waitpointId); + expect(caught.cause).toBe(classificationError); + }); +}); From 2f7fff63e4a109efa703d0d2e8ce1c0616f88793 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 14 Jul 2026 13:50:26 +0000 Subject: [PATCH 3/3] chore: format completeWaitpoint test to satisfy oxfmt --- .../tests/completeWaitpointErrorClassification.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal-packages/run-engine/src/engine/tests/completeWaitpointErrorClassification.test.ts b/internal-packages/run-engine/src/engine/tests/completeWaitpointErrorClassification.test.ts index d84b380a37..bc6ebd5f8d 100644 --- a/internal-packages/run-engine/src/engine/tests/completeWaitpointErrorClassification.test.ts +++ b/internal-packages/run-engine/src/engine/tests/completeWaitpointErrorClassification.test.ts @@ -54,9 +54,9 @@ describe("completeWaitpoint store-resolution error classification", () => { const classificationError = new UnclassifiableRunId(waitpointId); const waitpointSystem = createWaitpointSystem(() => Promise.reject(classificationError)); - await expect( - waitpointSystem.completeWaitpoint({ id: waitpointId }) - ).rejects.toBeInstanceOf(UnclassifiableWaitpointId); + await expect(waitpointSystem.completeWaitpoint({ id: waitpointId })).rejects.toBeInstanceOf( + UnclassifiableWaitpointId + ); const caught = (await waitpointSystem .completeWaitpoint({ id: waitpointId })