From 2b6418215a95240a7df76c75fda516a4febe53d4 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Thu, 2 Apr 2026 08:46:30 +0000 Subject: [PATCH 1/2] fix: populate innerFailure from error.cause in newFailureDetails() The newFailureDetails() function converts JavaScript errors to protobuf TaskFailureDetails but never populated the innerFailure field, even when the source error has a .cause property (ES2022 error chaining). The protobuf TaskFailureDetails message has a recursive innerFailure field specifically designed to preserve error chains. The codebase creates errors with { cause: ... } in 9+ places, but all that inner diagnostic information was being silently dropped. This change: - Recursively populates innerFailure from error.cause - Adds a depth limit (10) to guard against circular cause chains - Adds 6 new unit tests covering cause chains, non-Error causes, depth limits, and the no-cause case Fixes #210 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/utils/pb-helper.util.ts | 9 ++- .../test/pb-helper-versioning.spec.ts | 80 +++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/packages/durabletask-js/src/utils/pb-helper.util.ts b/packages/durabletask-js/src/utils/pb-helper.util.ts index dd03077..80aab37 100644 --- a/packages/durabletask-js/src/utils/pb-helper.util.ts +++ b/packages/durabletask-js/src/utils/pb-helper.util.ts @@ -185,7 +185,8 @@ export function newSubOrchestrationFailedEvent(eventId: number, ex: Error): pb.H return event; } -export function newFailureDetails(e: unknown): pb.TaskFailureDetails { +export function newFailureDetails(e: unknown, _depth: number = 0): pb.TaskFailureDetails { + const MAX_CAUSE_DEPTH = 10; const failure = new pb.TaskFailureDetails(); // Use e.name (which can be customized) over constructor.name (which is always the class name) // This allows users to set error.name = "CustomError" and have it preserved in failure details @@ -202,6 +203,12 @@ export function newFailureDetails(e: unknown): pb.TaskFailureDetails { failure.setStacktrace(sv); } + // Populate innerFailure from error.cause to preserve the full error chain. + // A depth limit guards against pathological circular cause chains. + if (e instanceof Error && e.cause && _depth < MAX_CAUSE_DEPTH) { + failure.setInnerfailure(newFailureDetails(e.cause, _depth + 1)); + } + return failure; } diff --git a/packages/durabletask-js/test/pb-helper-versioning.spec.ts b/packages/durabletask-js/test/pb-helper-versioning.spec.ts index 9331c4c..3ed20fb 100644 --- a/packages/durabletask-js/test/pb-helper-versioning.spec.ts +++ b/packages/durabletask-js/test/pb-helper-versioning.spec.ts @@ -66,5 +66,85 @@ describe("pb-helper.util - Version Mismatch Failure Details", () => { expect(failure.getErrortype()).toBe("CustomError"); expect(failure.getErrormessage()).toBe("Custom error message"); }); + + it("should populate innerFailure from error.cause", () => { + const innerError = new TypeError("Cannot read property 'x' of undefined"); + const outerError = new Error("Failed to process input", { cause: innerError }); + const failure = newFailureDetails(outerError); + + expect(failure.getErrortype()).toBe("Error"); + expect(failure.getErrormessage()).toBe("Failed to process input"); + + const inner = failure.getInnerfailure(); + expect(inner).toBeDefined(); + expect(inner!.getErrortype()).toBe("TypeError"); + expect(inner!.getErrormessage()).toBe("Cannot read property 'x' of undefined"); + expect(inner!.getStacktrace()).toBeDefined(); + }); + + it("should handle multi-level error cause chains", () => { + const rootCause = new RangeError("Index out of bounds"); + const midError = new Error("Data access failed", { cause: rootCause }); + const topError = new Error("Operation failed", { cause: midError }); + const failure = newFailureDetails(topError); + + expect(failure.getErrortype()).toBe("Error"); + expect(failure.getErrormessage()).toBe("Operation failed"); + + const mid = failure.getInnerfailure(); + expect(mid).toBeDefined(); + expect(mid!.getErrortype()).toBe("Error"); + expect(mid!.getErrormessage()).toBe("Data access failed"); + + const root = mid!.getInnerfailure(); + expect(root).toBeDefined(); + expect(root!.getErrortype()).toBe("RangeError"); + expect(root!.getErrormessage()).toBe("Index out of bounds"); + expect(root!.getInnerfailure()).toBeUndefined(); + }); + + it("should handle non-Error cause values", () => { + const outerError = new Error("Wrapper", { cause: "string cause" }); + const failure = newFailureDetails(outerError); + + const inner = failure.getInnerfailure(); + expect(inner).toBeDefined(); + expect(inner!.getErrortype()).toBe("UnknownError"); + expect(inner!.getErrormessage()).toBe("string cause"); + }); + + it("should not set innerFailure when there is no cause", () => { + const error = new Error("No cause"); + const failure = newFailureDetails(error); + + expect(failure.getInnerfailure()).toBeUndefined(); + }); + + it("should not set innerFailure for non-Error values", () => { + const failure = newFailureDetails("plain string error"); + + expect(failure.getErrortype()).toBe("UnknownError"); + expect(failure.getErrormessage()).toBe("plain string error"); + expect(failure.getInnerfailure()).toBeUndefined(); + }); + + it("should cap recursion depth to prevent stack overflow from circular causes", () => { + // Build a cause chain deeper than the limit (10) + let error: Error = new Error("root"); + for (let i = 0; i < 15; i++) { + error = new Error(`level-${i}`, { cause: error }); + } + + const failure = newFailureDetails(error); + + // Walk down the chain — should stop at depth 10 + let current = failure; + let depth = 0; + while (current.getInnerfailure()) { + current = current.getInnerfailure()!; + depth++; + } + expect(depth).toBe(10); + }); }); }); From 76ff1338d815397e72c41652120dfa6fbc641240 Mon Sep 17 00:00:00 2001 From: wangbill Date: Fri, 17 Jul 2026 15:45:55 -0700 Subject: [PATCH 2/2] Address review comments: guard falsy causes, hide recursion depth, rename test - newFailureDetails: guard error.cause with '!= null' instead of truthiness so a falsy-but-present cause (e.g. "" or 0) still populates innerFailure. - Extract recursion into a module-private buildFailureDetails() helper so the public newFailureDetails() signature no longer exposes the internal depth parameter. - Rename the depth-cap test to reflect a deep linear chain (not a cycle). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d175a43f-c852-49a2-a0de-63db579a8e3a --- .../durabletask-js/src/utils/pb-helper.util.ts | 17 ++++++++++++++--- .../test/pb-helper-versioning.spec.ts | 4 ++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/durabletask-js/src/utils/pb-helper.util.ts b/packages/durabletask-js/src/utils/pb-helper.util.ts index 80aab37..7551054 100644 --- a/packages/durabletask-js/src/utils/pb-helper.util.ts +++ b/packages/durabletask-js/src/utils/pb-helper.util.ts @@ -185,7 +185,16 @@ export function newSubOrchestrationFailedEvent(eventId: number, ex: Error): pb.H return event; } -export function newFailureDetails(e: unknown, _depth: number = 0): pb.TaskFailureDetails { +export function newFailureDetails(e: unknown): pb.TaskFailureDetails { + return buildFailureDetails(e, 0); +} + +/** + * Recursively builds TaskFailureDetails, populating innerFailure from error.cause. + * The depth parameter is internal to bound the cause-chain recursion and is not + * exposed on the public newFailureDetails() signature. + */ +function buildFailureDetails(e: unknown, depth: number): pb.TaskFailureDetails { const MAX_CAUSE_DEPTH = 10; const failure = new pb.TaskFailureDetails(); // Use e.name (which can be customized) over constructor.name (which is always the class name) @@ -205,8 +214,10 @@ export function newFailureDetails(e: unknown, _depth: number = 0): pb.TaskFailur // Populate innerFailure from error.cause to preserve the full error chain. // A depth limit guards against pathological circular cause chains. - if (e instanceof Error && e.cause && _depth < MAX_CAUSE_DEPTH) { - failure.setInnerfailure(newFailureDetails(e.cause, _depth + 1)); + // error.cause can be any value, so guard against null/undefined explicitly rather + // than truthiness — a falsy-but-present cause (e.g. "" or 0) is still a valid cause. + if (e instanceof Error && e.cause != null && depth < MAX_CAUSE_DEPTH) { + failure.setInnerfailure(buildFailureDetails(e.cause, depth + 1)); } return failure; diff --git a/packages/durabletask-js/test/pb-helper-versioning.spec.ts b/packages/durabletask-js/test/pb-helper-versioning.spec.ts index 3ed20fb..2fa544b 100644 --- a/packages/durabletask-js/test/pb-helper-versioning.spec.ts +++ b/packages/durabletask-js/test/pb-helper-versioning.spec.ts @@ -128,8 +128,8 @@ describe("pb-helper.util - Version Mismatch Failure Details", () => { expect(failure.getInnerfailure()).toBeUndefined(); }); - it("should cap recursion depth to prevent stack overflow from circular causes", () => { - // Build a cause chain deeper than the limit (10) + it("should cap recursion depth for deep cause chains", () => { + // Build a linear cause chain deeper than the limit (10) let error: Error = new Error("root"); for (let i = 0; i < 15; i++) { error = new Error(`level-${i}`, { cause: error });