From fd684cf77e9dde62c0efa0f81b1b20b9f8710459 Mon Sep 17 00:00:00 2001 From: wangbill Date: Wed, 23 Sep 2026 13:05:39 -0700 Subject: [PATCH 1/2] fix: preserve nested task failure details Expose innerFailure throughout task errors, retry inspection, client state, history and in-memory testing. Keep received details below TaskFailedError when an uncaught or rethrown failure is serialized. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e0af01a5-0dfa-4e71-a660-c4186e65d7e0 --- CHANGELOG.md | 4 + README.md | 33 +++ packages/azure-functions-durable/CHANGELOG.md | 1 + .../src/testing/index.ts | 1 + .../test/unit/testing.spec.ts | 7 +- packages/durabletask-js/src/client/client.ts | 12 +- .../durabletask-js/src/orchestration/index.ts | 13 +- .../src/task/exception/task-failed-error.ts | 7 +- .../src/task/failure-details.ts | 13 +- .../src/task/retry-handler-task.ts | 9 +- .../src/task/retry/retry-context.ts | 2 +- .../durabletask-js/src/task/retryable-task.ts | 9 +- .../src/testing/in-memory-backend.ts | 2 +- .../durabletask-js/src/testing/test-client.ts | 11 +- .../src/utils/failure-details.util.ts | 20 ++ .../src/utils/history-event-converter.ts | 12 +- .../src/utils/pb-helper.util.ts | 32 ++- .../test/failure-details-grpc.spec.ts | 98 ++++++++ .../test/failure-details.spec.ts | 234 ++++++++++++++++++ .../test/retry-handler-task.spec.ts | 48 ++++ 20 files changed, 503 insertions(+), 65 deletions(-) create mode 100644 packages/durabletask-js/src/utils/failure-details.util.ts create mode 100644 packages/durabletask-js/test/failure-details-grpc.spec.ts create mode 100644 packages/durabletask-js/test/failure-details.spec.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index fa7c3a65..c9ebdfe4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,10 @@ ### Fixes +- Preserve nested `innerFailure` details in task errors, retry handlers and predicates, + client state/history, and in-memory testing. Uncaught or rethrown task failures retain + their received chain beneath the existing `TaskFailedError` wrapper. Retry decisions + and the ordinary `Error.cause` serialization depth limit are unchanged. - Preserve explicit empty activity and child versions on the protobuf wire, including the unversioned child default, so concrete empty-version work-item filters can match them. - Reject uninitialized `whenAll` results after a canceled child's completion callback throws, diff --git a/README.md b/README.md index c09de53c..8373fc53 100644 --- a/README.md +++ b/README.md @@ -187,6 +187,39 @@ User code and metadata generation are not canceled; if metadata finishes after s the response RPC is not started. Channel retirement and backend lock durations are unchanged. Retries do not guarantee connection recovery, acceptance of expired tokens, or exactly-once execution. +### Inspecting nested failures + +Task errors expose an optional, read-only `innerFailure` chain, with `errorType`, `message`, +and optional `stackTrace` at each level. For an activity failure +`OrderFailed -> PaymentFailed -> ConnectionTimeout`, orchestrator code can inspect: + +```typescript +import { TaskFailedError } from "@microsoft/durabletask-js"; + +try { + yield ctx.callActivity("placeOrder"); +} catch (error) { + if (error instanceof TaskFailedError) { + const paymentFailure = error.details.innerFailure; + const rootFailure = paymentFailure?.innerFailure; + ctx.setCustomStatus({ rootErrorType: rootFailure?.errorType }); + } + throw error; +} +``` + +The same chain is available as `context.lastFailure.innerFailure` in retry handlers and +`failure.innerFailure` in `RetryPolicy.handleFailure`. Inspecting nested failures does not +change retry decisions automatically. Client state and history also preserve the chain; +an uncaught or rethrown task error retains its `TaskFailedError` wrapper, with the original +task details under `state.failureDetails.innerFailure`. + +Missing inner failures remain `undefined`; existing two- and three-argument +`FailureDetails` constructors still work, with an optional fourth argument for the inner +details. Remote failures are not reconstructed as JavaScript `Error.cause` objects. +The existing writer still limits ordinary `Error.cause` chains to ten inner levels; +reading or forwarding already-received failure details does not add a truncation limit. + ### Reusing orchestration instance IDs Set the top-level `dedupeStatuses` start option when an instance ID may be reused. The list diff --git a/packages/azure-functions-durable/CHANGELOG.md b/packages/azure-functions-durable/CHANGELOG.md index 83b5e352..b1543fda 100644 --- a/packages/azure-functions-durable/CHANGELOG.md +++ b/packages/azure-functions-durable/CHANGELOG.md @@ -14,6 +14,7 @@ ### Fixes +- Preserve nested task `innerFailure` details in `durable-functions/testing` results. - Inherit core version-aware replay dispatch and worker child defaults in the embedded `DurableFunctionsWorker`, including classic-context wrappers. Host `app.*` function registrations remain name-only; see README for the boundary and activity-version migration guidance. diff --git a/packages/azure-functions-durable/src/testing/index.ts b/packages/azure-functions-durable/src/testing/index.ts index 6111564a..10777df1 100644 --- a/packages/azure-functions-durable/src/testing/index.ts +++ b/packages/azure-functions-durable/src/testing/index.ts @@ -97,6 +97,7 @@ export async function runOrchestrator( errorType: state.failureDetails.errorType, message: state.failureDetails.message, stackTrace: state.failureDetails.stackTrace, + innerFailure: state.failureDetails.innerFailure, } : undefined, }; diff --git a/packages/azure-functions-durable/test/unit/testing.spec.ts b/packages/azure-functions-durable/test/unit/testing.spec.ts index 00bf5861..202089fb 100644 --- a/packages/azure-functions-durable/test/unit/testing.spec.ts +++ b/packages/azure-functions-durable/test/unit/testing.spec.ts @@ -141,7 +141,7 @@ describe("durable-functions/testing", () => { const result = await runOrchestrator(orchestrator, { activities: { fail: () => { - throw new TypeError("activity failed"); + throw new TypeError("activity failed", { cause: new Error("connection timed out") }); }, }, }); @@ -151,6 +151,11 @@ describe("durable-functions/testing", () => { expect(result.failure).toMatchObject({ errorType: expect.any(String), message: expect.stringContaining("activity failed"), + innerFailure: { + errorType: "TypeError", + message: "activity failed", + innerFailure: { errorType: "Error", message: "connection timed out", innerFailure: undefined }, + }, }); }); diff --git a/packages/durabletask-js/src/client/client.ts b/packages/durabletask-js/src/client/client.ts index 11bbe349..38567b0f 100644 --- a/packages/durabletask-js/src/client/client.ts +++ b/packages/durabletask-js/src/client/client.ts @@ -24,7 +24,7 @@ import { TerminateInstanceOptions, isTerminateInstanceOptions } from "../orchest import { callWithMetadata, MetadataGenerator } from "../utils/grpc-helper.util"; import { OrchestrationQuery, ListInstanceIdsOptions, DEFAULT_PAGE_SIZE } from "../orchestration/orchestration-query"; import { Page, AsyncPageable, createAsyncPageable } from "../orchestration/page"; -import { FailureDetails } from "../task/failure-details"; +import { convertFailureDetails } from "../utils/failure-details.util"; import { HistoryEvent } from "../orchestration/history-event"; import { convertProtoHistoryEvent } from "../utils/history-event-converter"; import { Logger, ConsoleLogger } from "../types/logger.type"; @@ -1376,15 +1376,7 @@ export class TaskHubGrpcClient { } // Extract failure details if present - let failureDetails; - const protoFailureDetails = protoState.getFailuredetails(); - if (protoFailureDetails) { - failureDetails = new FailureDetails( - protoFailureDetails.getErrormessage(), - protoFailureDetails.getErrortype(), - protoFailureDetails.getStacktrace()?.getValue(), - ); - } + const failureDetails = convertFailureDetails(protoState.getFailuredetails()); const tags = mapToRecord(protoState.getTagsMap()); diff --git a/packages/durabletask-js/src/orchestration/index.ts b/packages/durabletask-js/src/orchestration/index.ts index cbb17cc0..9b9035ae 100644 --- a/packages/durabletask-js/src/orchestration/index.ts +++ b/packages/durabletask-js/src/orchestration/index.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import * as pb from "../proto/orchestrator_service_pb"; -import { FailureDetails } from "../task/failure-details"; +import { convertFailureDetails } from "../utils/failure-details.util"; import { fromProtobuf } from "./enum/orchestration-status.enum"; import { OrchestrationState } from "./orchestration-state"; import { mapToRecord } from "../utils/tags.util"; @@ -16,16 +16,7 @@ export function newOrchestrationState( } const state = res.getOrchestrationstate(); - let failureDetails; - - const protoFailureDetails = state?.getFailuredetails(); - if (protoFailureDetails) { - failureDetails = new FailureDetails( - protoFailureDetails.getErrormessage(), - protoFailureDetails.getErrortype(), - protoFailureDetails.getStacktrace()?.getValue(), - ); - } + const failureDetails = convertFailureDetails(state?.getFailuredetails()); const tsCreated = state?.getCreatedtimestamp(); const tsUpdated = state?.getLastupdatedtimestamp(); diff --git a/packages/durabletask-js/src/task/exception/task-failed-error.ts b/packages/durabletask-js/src/task/exception/task-failed-error.ts index 030ab52b..1780d5db 100644 --- a/packages/durabletask-js/src/task/exception/task-failed-error.ts +++ b/packages/durabletask-js/src/task/exception/task-failed-error.ts @@ -3,6 +3,7 @@ import * as pb from "../../proto/orchestrator_service_pb"; import { FailureDetails } from "../failure-details"; +import { convertFailureDetails } from "../../utils/failure-details.util"; export class TaskFailedError extends Error { private _details: FailureDetails; @@ -11,11 +12,7 @@ export class TaskFailedError extends Error { super(message); this.name = "TaskFailedError"; - this._details = new FailureDetails( - details.getErrormessage(), - details.getErrortype(), - details?.getStacktrace()?.getValue(), - ); + this._details = convertFailureDetails(details); } get details(): FailureDetails { diff --git a/packages/durabletask-js/src/task/failure-details.ts b/packages/durabletask-js/src/task/failure-details.ts index 2dec0d7d..83ee7031 100644 --- a/packages/durabletask-js/src/task/failure-details.ts +++ b/packages/durabletask-js/src/task/failure-details.ts @@ -3,7 +3,8 @@ /** * Interface representing task failure details. - * This is used for retry handlers to inspect failure information. + * Used by task errors and retry handlers to inspect failure information, + * including the optional chain of underlying failures. */ export interface TaskFailureDetails { /** The type/class name of the error */ @@ -12,17 +13,21 @@ export interface TaskFailureDetails { readonly message: string; /** The stack trace, if available */ readonly stackTrace?: string; + /** Details of the underlying failure, if supplied by the task or backend. */ + readonly innerFailure?: TaskFailureDetails; } export class FailureDetails implements TaskFailureDetails { private _message: string; private _errorType: string; private _stackTrace: string | undefined; + private _innerFailure: TaskFailureDetails | undefined; - constructor(message: string, errorType: string, stackTrace?: string) { + constructor(message: string, errorType: string, stackTrace?: string, innerFailure?: TaskFailureDetails) { this._message = message; this._errorType = errorType; this._stackTrace = stackTrace; + this._innerFailure = innerFailure; } get message(): string { @@ -36,4 +41,8 @@ export class FailureDetails implements TaskFailureDetails { get stackTrace(): string | undefined { return this._stackTrace; } + + get innerFailure(): TaskFailureDetails | undefined { + return this._innerFailure; + } } diff --git a/packages/durabletask-js/src/task/retry-handler-task.ts b/packages/durabletask-js/src/task/retry-handler-task.ts index 68024dd5..2caa1408 100644 --- a/packages/durabletask-js/src/task/retry-handler-task.ts +++ b/packages/durabletask-js/src/task/retry-handler-task.ts @@ -7,6 +7,7 @@ import { RetryTaskBase, RetryTaskType } from "./retry-task-base"; import { AsyncRetryHandler, RetryHandlerResult } from "./retry/retry-handler"; import { createRetryContext } from "./retry/retry-context"; import { TaskFailureDetails } from "./failure-details"; +import { convertFailureDetails } from "../utils/failure-details.util"; /** * A task that uses an AsyncRetryHandler for imperative retry control. @@ -73,10 +74,12 @@ export class RetryHandlerTask extends RetryTaskBase { return false; } + const details = convertFailureDetails(this.lastFailure); const failureDetails: TaskFailureDetails = { - errorType: this.lastFailure.getErrortype() || "Error", - message: this.lastFailure.getErrormessage() || "", - stackTrace: this.lastFailure.getStacktrace()?.getValue(), + errorType: details.errorType || "Error", + message: details.message, + stackTrace: details.stackTrace, + innerFailure: details.innerFailure, }; const totalRetryTimeMs = currentTime.getTime() - this.startTime.getTime(); diff --git a/packages/durabletask-js/src/task/retry/retry-context.ts b/packages/durabletask-js/src/task/retry/retry-context.ts index c33217d7..1d6795af 100644 --- a/packages/durabletask-js/src/task/retry/retry-context.ts +++ b/packages/durabletask-js/src/task/retry/retry-context.ts @@ -59,7 +59,7 @@ export interface RetryContext { /** * The details of the previous task failure. - * Contains the error type, message, and stack trace. + * Contains the error type, message, stack trace, and optional inner failure chain. */ readonly lastFailure: TaskFailureDetails; diff --git a/packages/durabletask-js/src/task/retryable-task.ts b/packages/durabletask-js/src/task/retryable-task.ts index d1d48556..00028249 100644 --- a/packages/durabletask-js/src/task/retryable-task.ts +++ b/packages/durabletask-js/src/task/retryable-task.ts @@ -4,6 +4,7 @@ import { RetryTaskBase, RetryTaskType } from "./retry-task-base"; import { RetryPolicy } from "./retry/retry-policy"; import * as pb from "../proto/orchestrator_service_pb"; +import { convertFailureDetails } from "../utils/failure-details.util"; /** * A task that can be retried according to a declarative retry policy. @@ -66,10 +67,12 @@ export class RetryableTask extends RetryTaskBase { // Check if handleFailure predicate says we should NOT retry this failure type if (this.lastFailure) { + const details = convertFailureDetails(this.lastFailure); const failureDetails = { - errorType: this.lastFailure.getErrortype() || "Error", - message: this.lastFailure.getErrormessage() || "", - stackTrace: this.lastFailure.getStacktrace()?.getValue(), + errorType: details.errorType || "Error", + message: details.message, + stackTrace: details.stackTrace, + innerFailure: details.innerFailure, }; if (!this._retryPolicy.shouldRetry(failureDetails)) { diff --git a/packages/durabletask-js/src/testing/in-memory-backend.ts b/packages/durabletask-js/src/testing/in-memory-backend.ts index 737c1226..0859cb23 100644 --- a/packages/durabletask-js/src/testing/in-memory-backend.ts +++ b/packages/durabletask-js/src/testing/in-memory-backend.ts @@ -1245,7 +1245,7 @@ export class InMemoryOrchestrationBackend { event = pbh.newSubOrchestrationCompletedEvent(taskId, subInstance.output); } else { const error = new Error(subInstance.failureDetails?.getErrormessage() || "Sub-orchestration failed"); - event = pbh.newSubOrchestrationFailedEvent(taskId, error); + event = pbh.newSubOrchestrationFailedEvent(taskId, error, subInstance.failureDetails); } parentInstance.pendingEvents.push(event); parentInstance.lastUpdatedAt = new Date(); diff --git a/packages/durabletask-js/src/testing/test-client.ts b/packages/durabletask-js/src/testing/test-client.ts index cbb45d0b..14a93832 100644 --- a/packages/durabletask-js/src/testing/test-client.ts +++ b/packages/durabletask-js/src/testing/test-client.ts @@ -6,7 +6,7 @@ import { getName } from "../task"; import { TOrchestrator } from "../types/orchestrator.type"; import { TInput } from "../types/input.type"; import { OrchestrationState } from "../orchestration/orchestration-state"; -import { FailureDetails } from "../task/failure-details"; +import { convertFailureDetails } from "../utils/failure-details.util"; import { EntityInstanceId } from "../entities/entity-instance-id"; import { EntityMetadata } from "../entities/entity-metadata"; import { InMemoryOrchestrationBackend, OrchestrationInstance } from "./in-memory-backend"; @@ -229,14 +229,7 @@ export class TestOrchestrationClient { } private toOrchestrationState(instance: OrchestrationInstance, fetchPayloads: boolean): OrchestrationState { - let failureDetails: FailureDetails | undefined; - if (instance.failureDetails) { - failureDetails = new FailureDetails( - instance.failureDetails.getErrormessage(), - instance.failureDetails.getErrortype(), - instance.failureDetails.getStacktrace()?.getValue(), - ); - } + const failureDetails = convertFailureDetails(instance.failureDetails); return new OrchestrationState( instance.instanceId, diff --git a/packages/durabletask-js/src/utils/failure-details.util.ts b/packages/durabletask-js/src/utils/failure-details.util.ts new file mode 100644 index 00000000..7d2ce7f1 --- /dev/null +++ b/packages/durabletask-js/src/utils/failure-details.util.ts @@ -0,0 +1,20 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as pb from "../proto/orchestrator_service_pb"; +import { FailureDetails } from "../task/failure-details"; + +export function convertFailureDetails(details: pb.TaskFailureDetails): FailureDetails; +export function convertFailureDetails(details: pb.TaskFailureDetails | undefined): FailureDetails | undefined; +export function convertFailureDetails(details: pb.TaskFailureDetails | undefined): FailureDetails | undefined { + if (!details) { + return undefined; + } + + return new FailureDetails( + details.getErrormessage(), + details.getErrortype(), + details.getStacktrace()?.getValue(), + convertFailureDetails(details.getInnerfailure()), + ); +} diff --git a/packages/durabletask-js/src/utils/history-event-converter.ts b/packages/durabletask-js/src/utils/history-event-converter.ts index 1d33bf2f..3798c953 100644 --- a/packages/durabletask-js/src/utils/history-event-converter.ts +++ b/packages/durabletask-js/src/utils/history-event-converter.ts @@ -9,7 +9,7 @@ import { ParentInstanceInfo, TraceContext, } from "../orchestration/history-event"; -import { FailureDetails } from "../task/failure-details"; +import { convertFailureDetails } from "./failure-details.util"; // Map OrchestrationStatus enum values to their string names const ORCHESTRATION_STATUS_MAP: Record = { @@ -498,16 +498,6 @@ function convertTraceContext(traceContext: pb.TraceContext): TraceContext { }; } -function convertFailureDetails(details: pb.TaskFailureDetails | undefined): FailureDetails | undefined { - if (!details) return undefined; - - return new FailureDetails( - details.getErrormessage(), - details.getErrortype(), - details.getStacktrace()?.getValue(), - ); -} - function convertTagsMap(tagsMap: ReturnType): Record | undefined { const result: Record = {}; let hasEntries = false; diff --git a/packages/durabletask-js/src/utils/pb-helper.util.ts b/packages/durabletask-js/src/utils/pb-helper.util.ts index a62fba5c..d273f042 100644 --- a/packages/durabletask-js/src/utils/pb-helper.util.ts +++ b/packages/durabletask-js/src/utils/pb-helper.util.ts @@ -6,6 +6,8 @@ import * as pb from "../proto/orchestrator_service_pb"; import { Timestamp } from "google-protobuf/google/protobuf/timestamp_pb"; import { ActivityNotRegisteredError } from "../worker/exception/activity-not-registered-error"; import { OrchestratorNotRegisteredError } from "../worker/exception/orchestrator-not-registered-error"; +import { TaskFailedError } from "../task/exception/task-failed-error"; +import { TaskFailureDetails } from "../task/failure-details"; export function newOrchestratorStartedEvent(timestamp?: Date | null): pb.HistoryEvent { const ts = new Timestamp(); @@ -187,11 +189,15 @@ export function newSubOrchestrationCompletedEvent(eventId: number, encodedOutput return event; } -export function newSubOrchestrationFailedEvent(eventId: number, ex: Error): pb.HistoryEvent { +export function newSubOrchestrationFailedEvent( + eventId: number, + ex: Error, + failureDetails?: pb.TaskFailureDetails, +): pb.HistoryEvent { const ts = new Timestamp(); const subOrchestrationInstanceFailedEvent = new pb.SubOrchestrationInstanceFailedEvent(); - subOrchestrationInstanceFailedEvent.setFailuredetails(newFailureDetails(ex)); + subOrchestrationInstanceFailedEvent.setFailuredetails(failureDetails ?? newFailureDetails(ex)); subOrchestrationInstanceFailedEvent.setTaskscheduledid(eventId); const event = new pb.HistoryEvent(); @@ -207,7 +213,7 @@ export function newFailureDetails(e: unknown): pb.TaskFailureDetails { } /** - * Recursively builds TaskFailureDetails, populating innerFailure from error.cause. + * Builds TaskFailureDetails, forwarding received task details or serializing error.cause. * The depth parameter is internal to bound the cause-chain recursion and is not * exposed on the public newFailureDetails() signature. */ @@ -232,11 +238,21 @@ function buildFailureDetails(e: unknown, depth: number): 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. - // 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) { + if (e instanceof TaskFailedError && e.cause == null) { + // Keep the task wrapper and attach the received failure chain without rebuilding Error objects. + let current = failure; + for (let details: TaskFailureDetails | undefined = e.details; details; details = details.innerFailure) { + const inner = new pb.TaskFailureDetails(); + inner.setErrortype(details.errorType); + inner.setErrormessage(details.message); + if (details.stackTrace !== undefined) { + inner.setStacktrace(new StringValue().setValue(details.stackTrace)); + } + current.setInnerfailure(inner); + current = inner; + } + } else if (e instanceof Error && e.cause != null && depth < MAX_CAUSE_DEPTH) { + // Bound ordinary cause chains, including cycles; falsy-but-present causes remain valid. failure.setInnerfailure(buildFailureDetails(e.cause, depth + 1)); } diff --git a/packages/durabletask-js/test/failure-details-grpc.spec.ts b/packages/durabletask-js/test/failure-details-grpc.spec.ts new file mode 100644 index 00000000..16088f1e --- /dev/null +++ b/packages/durabletask-js/test/failure-details-grpc.spec.ts @@ -0,0 +1,98 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as grpc from "@grpc/grpc-js"; +import { Empty } from "google-protobuf/google/protobuf/empty_pb"; +import { NoOpLogger, TaskHubGrpcClient, TaskHubGrpcWorker } from "../src"; +import * as pb from "../src/proto/orchestrator_service_pb"; +import * as stubs from "../src/proto/orchestrator_service_grpc_pb"; +import { withTimeout } from "../src/utils/backoff.util"; + +it("preserves serialized worker failures through gRPC client get, waits, query and history", async () => { + const server = new grpc.Server(); + const state = new pb.OrchestrationState() + .setInstanceid("order") + .setName("order") + .setOrchestrationstatus(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED); + const response = new pb.GetInstanceResponse().setExists(true).setOrchestrationstate(state); + const get: grpc.handleUnaryCall = (_call, callback) => + callback(null, response); + let completed!: () => void; + const activityCompleted = new Promise((resolve) => (completed = resolve)); + server.addService(stubs.TaskHubSidecarServiceService, { + hello: (_call, callback) => callback(null, new Empty()), + getWorkItems: (call) => { + call.once("cancelled", () => call.end()); + call.write( + new pb.WorkItem() + .setCompletiontoken("activity") + .setActivityrequest( + new pb.ActivityRequest() + .setName("fail") + .setTaskid(1) + .setOrchestrationinstance(new pb.OrchestrationInstance().setInstanceid("order")), + ), + ); + }, + completeActivityTask: (call, callback) => { + state.setFailuredetails(call.request.getFailuredetails()); + callback(null, new pb.CompleteTaskResponse()); + completed(); + }, + getInstance: get, + waitForInstanceStart: get, + waitForInstanceCompletion: get, + queryInstances: (_call, callback) => + callback(null, new pb.QueryInstancesResponse().setOrchestrationstateList([state])), + streamInstanceHistory: (call) => { + call.write( + new pb.HistoryChunk().setEventsList([ + new pb.HistoryEvent().setTaskfailed( + new pb.TaskFailedEvent().setTaskscheduledid(1).setFailuredetails(state.getFailuredetails()), + ), + ]), + ); + call.end(); + }, + } satisfies Partial); + const port = await new Promise((resolve, reject) => { + server.bindAsync("127.0.0.1:0", grpc.ServerCredentials.createInsecure(), (error, boundPort) => + error ? reject(error) : resolve(boundPort), + ); + }); + const options = { hostAddress: `127.0.0.1:${port}`, logger: new NoOpLogger() }; + const worker = new TaskHubGrpcWorker({ ...options, shutdownTimeoutMs: 500 }); + const client = new TaskHubGrpcClient(options); + worker.addNamedActivity("fail", () => { + throw new Error("Order failed", { cause: new TypeError("Payment failed", { cause: new RangeError("Timeout") }) }); + }); + await worker.start(); + try { + await withTimeout(activityCompleted, 5000); + const expected = { + errorType: "Error", + message: "Order failed", + innerFailure: { + errorType: "TypeError", + message: "Payment failed", + innerFailure: { errorType: "RangeError", message: "Timeout", innerFailure: undefined }, + }, + }; + for (const method of [ + "getOrchestrationState", + "waitForOrchestrationStart", + "waitForOrchestrationCompletion", + ] as const) { + expect((await client[method]("order", false))?.failureDetails).toMatchObject(expected); + } + const states = []; + for await (const instance of client.getAllInstances()) states.push(instance); + expect(states).toHaveLength(1); + expect(states[0].failureDetails).toMatchObject(expected); + expect(await client.getOrchestrationHistory("order")).toMatchObject([{ failureDetails: expected }]); + } finally { + await client.stop(); + await worker.stop(); + server.forceShutdown(); + } +}); diff --git a/packages/durabletask-js/test/failure-details.spec.ts b/packages/durabletask-js/test/failure-details.spec.ts new file mode 100644 index 00000000..ad3bd053 --- /dev/null +++ b/packages/durabletask-js/test/failure-details.spec.ts @@ -0,0 +1,234 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { StringValue } from "google-protobuf/google/protobuf/wrappers_pb"; +import { + FailureDetails, + InMemoryOrchestrationBackend, + OrchestrationStatus, + RetryPolicy, + TaskFailedError, + TaskFailureDetails, + TestOrchestrationClient, + TestOrchestrationWorker, +} from "../src"; +import { newOrchestrationState } from "../src/orchestration"; +import * as pb from "../src/proto/orchestrator_service_pb"; +import { convertProtoHistoryEvent } from "../src/utils/history-event-converter"; +import { newFailureDetails } from "../src/utils/pb-helper.util"; + +function errorChain(): Error { + const inner = new Error("Connection timed out"); + inner.name = "ConnectionTimeout"; + inner.stack = undefined; + const middle = new Error("Payment failed", { cause: inner }); + middle.name = "PaymentFailed"; + middle.stack = ""; + const outer = new Error("Order failed", { cause: middle }); + outer.name = "OrderFailed"; + outer.stack = "order stack"; + return outer; +} + +const expectedChain = { + errorType: "OrderFailed", + message: "Order failed", + stackTrace: "order stack", + innerFailure: { + errorType: "PaymentFailed", + message: "Payment failed", + stackTrace: "", + innerFailure: { + errorType: "ConnectionTimeout", + message: "Connection timed out", + stackTrace: undefined, + innerFailure: undefined, + }, + }, +}; + +describe("Public failure details", () => { + it("preserves a three-level wire chain in TaskFailedError without changing its message or cause", () => { + const error = new TaskFailedError("Activity task failed", newFailureDetails(errorChain())); + expect(error.details).toMatchObject(expectedChain); + expect(error.name).toBe("TaskFailedError"); + expect(error.message).toBe("Activity task failed"); + expect(error.cause).toBeUndefined(); + }); + + it("keeps existing constructors and missing or empty wire fields compatible", () => { + expect(new FailureDetails("message", "Error", "stack")).toMatchObject({ + message: "message", + errorType: "Error", + stackTrace: "stack", + innerFailure: undefined, + }); + expect(new FailureDetails("message", "Error").stackTrace).toBeUndefined(); + const proto = new pb.TaskFailureDetails(); + expect(new TaskFailedError("failed", proto).details).toMatchObject({ + message: "", + errorType: "", + stackTrace: undefined, + innerFailure: undefined, + }); + proto.setStacktrace(new StringValue().setValue("")); + expect(new TaskFailedError("failed", proto).details.stackTrace).toBe(""); + const inner = new FailureDetails("inner", "InnerError"); + expect(new FailureDetails("outer", "OuterError", undefined, inner).innerFailure).toBe(inner); + }); + + it("does not apply the outbound Error.cause depth limit to an incoming wire chain", () => { + let proto = new pb.TaskFailureDetails().setErrortype("").setErrormessage(""); + let expected: object = { errorType: "", message: "", innerFailure: undefined }; + for (let i = 0; i < 15; i++) { + proto = new pb.TaskFailureDetails().setErrortype("Error").setErrormessage(String(i)).setInnerfailure(proto); + expected = { errorType: "Error", message: String(i), innerFailure: expected }; + } + const decoded = pb.TaskFailureDetails.deserializeBinary(proto.serializeBinary()); + const error = new TaskFailedError("failed", decoded); + expect(error.details).toMatchObject(expected); + const forwarded = newFailureDetails(error); + expect(forwarded.getErrortype()).toBe("TaskFailedError"); + expect(forwarded.getErrormessage()).toBe(error.message); + expect(forwarded.getStacktrace()?.getValue()).toBe(error.stack); + expect(forwarded.getInnerfailure()?.serializeBinary()).toEqual(proto.serializeBinary()); + }); + + it("retains explicitly assigned JavaScript causes on task errors", () => { + const error = new TaskFailedError("failed", newFailureDetails(errorChain())); + error.cause = new Error("explicit cause"); + expect(newFailureDetails(error).getInnerfailure()?.getErrormessage()).toBe("explicit cause"); + }); + + it("preserves the chain in orchestration state and raiseIfFailed", () => { + const proto = new pb.OrchestrationState().setFailuredetails(newFailureDetails(errorChain())); + const state = newOrchestrationState( + "order", + new pb.GetInstanceResponse().setExists(true).setOrchestrationstate(proto), + ); + expect(state?.failureDetails).toMatchObject(expectedChain); + let thrown: unknown; + try { + state?.raiseIfFailed(); + } catch (error) { + thrown = error; + } + expect(thrown).toMatchObject({ + name: "OrchestrationFailedError", + message: "Orchestration 'order' failed: Order failed", + failureDetails: expectedChain, + }); + }); + + it.each(["execution", "activity", "subOrchestration", "entity"] as const)( + "preserves the chain in %s history events", + (kind) => { + const details = newFailureDetails(errorChain()); + const event = new pb.HistoryEvent(); + if (kind === "execution") + event.setExecutioncompleted(new pb.ExecutionCompletedEvent().setFailuredetails(details)); + if (kind === "activity") event.setTaskfailed(new pb.TaskFailedEvent().setFailuredetails(details)); + if (kind === "subOrchestration") { + event.setSuborchestrationinstancefailed( + new pb.SubOrchestrationInstanceFailedEvent().setFailuredetails(details), + ); + } + if (kind === "entity") + event.setEntityoperationfailed(new pb.EntityOperationFailedEvent().setFailuredetails(details)); + expect(convertProtoHistoryEvent(event)).toMatchObject({ failureDetails: expectedChain }); + }, + ); +}); + +describe.each(["activity", "subOrchestration"] as const)("In-memory %s failure chains", (kind) => { + it.each(["handler", "policy"] as const)( + "reaches %s retry inspection, catch, history and client state", + async (mode) => { + const backend = new InMemoryOrchestrationBackend(); + const client = new TestOrchestrationClient(backend); + const worker = new TestOrchestrationWorker(backend); + const inspected: TaskFailureDetails[] = []; + const caught: FailureDetails[] = []; + const inspectFailure = (failure: TaskFailureDetails) => { + inspected.push(failure); + return false; + }; + const retry = + mode === "policy" + ? new RetryPolicy({ + firstRetryIntervalInMilliseconds: 1, + maxNumberOfAttempts: 3, + handleFailure: inspectFailure, + }) + : (context: { lastFailure: TaskFailureDetails }) => inspectFailure(context.lastFailure); + const fail = () => { + throw errorChain(); + }; + worker.addNamedActivity("fail", fail); + worker.addNamedOrchestrator("fail", fail); + worker.addNamedOrchestrator("order", async function* (ctx) { + try { + if (kind === "activity") yield ctx.callActivity("fail", undefined, { retry }); + else yield ctx.callSubOrchestrator("fail", undefined, { retry, instanceId: "child" }); + } catch (error) { + if (!(error instanceof TaskFailedError)) throw error; + caught.push(error.details); + throw error; + } + }); + await worker.start(); + try { + const id = await client.scheduleNewOrchestration("order"); + const state = await client.waitForOrchestrationCompletion(id, true, 5); + expect(inspected).toHaveLength(1); + expect(inspected[0]).toMatchObject(expectedChain); + expect(inspected[0].innerFailure?.innerFailure?.errorType).toBe("ConnectionTimeout"); + expect(caught).toHaveLength(1); + expect(caught[0]).toMatchObject(expectedChain); + expect(state?.runtimeStatus).toBe(OrchestrationStatus.FAILED); + const terminalFailure = { + errorType: "TaskFailedError", + innerFailure: expectedChain, + }; + expect(state?.failureDetails).toMatchObject(terminalFailure); + expect((await client.getOrchestrationState(id, false))?.failureDetails).toMatchObject(terminalFailure); + const history = backend.getInstance(id)!.history.map(convertProtoHistoryEvent); + const failedEvent = history.find( + (event) => event && "failureDetails" in event && event.failureDetails?.errorType === "OrderFailed", + ); + expect(failedEvent).toMatchObject({ failureDetails: expectedChain }); + if (kind === "subOrchestration") { + expect((await client.getOrchestrationState("child"))?.failureDetails).toMatchObject(expectedChain); + } + } finally { + await worker.stop(); + backend.reset(); + } + }, + ); +}); + +it("preserves an uncaught activity cause chain in the terminal client failure", async () => { + const backend = new InMemoryOrchestrationBackend(); + const client = new TestOrchestrationClient(backend); + const worker = new TestOrchestrationWorker(backend); + worker.addNamedActivity("fail", () => { + throw errorChain(); + }); + worker.addNamedOrchestrator("order", async function* (ctx) { + yield ctx.callActivity("fail"); + }); + await worker.start(); + try { + const id = await client.scheduleNewOrchestration("order"); + const state = await client.waitForOrchestrationCompletion(id, true, 5); + expect(state?.failureDetails).toMatchObject({ + errorType: "TaskFailedError", + message: "Activity task #1 failed: Order failed", + innerFailure: expectedChain, + }); + } finally { + await worker.stop(); + backend.reset(); + } +}); diff --git a/packages/durabletask-js/test/retry-handler-task.spec.ts b/packages/durabletask-js/test/retry-handler-task.spec.ts index cad9595c..a3c1fd5e 100644 --- a/packages/durabletask-js/test/retry-handler-task.spec.ts +++ b/packages/durabletask-js/test/retry-handler-task.spec.ts @@ -6,6 +6,9 @@ import { AsyncRetryHandler } from "../src/task/retry/retry-handler"; import { OrchestrationContext } from "../src/task/context/orchestration-context"; import * as pb from "../src/proto/orchestrator_service_pb"; import { StringValue } from "google-protobuf/google/protobuf/wrappers_pb"; +import { RetryableTask } from "../src/task/retryable-task"; +import { RetryPolicy } from "../src/task/retry/retry-policy"; +import { TaskFailureDetails } from "../src/task/failure-details"; /** * Creates a minimal mock OrchestrationContext for testing. @@ -187,6 +190,51 @@ describe("RetryHandlerTask", () => { }); describe("shouldRetry", () => { + it.each(["activity", "subOrchestration"] as const)( + "keeps %s retry defaults and only checks the top-level non-retriable flag", + async (kind) => { + const inspect = jest.fn((failure: TaskFailureDetails) => { + expect(failure).toMatchObject({ + errorType: "Error", + message: "", + innerFailure: { errorType: "", message: "", innerFailure: undefined }, + }); + return true; + }); + const action = new pb.OrchestratorAction(); + const startTime = new Date(); + const handlerTask = new RetryHandlerTask( + async (context) => inspect(context.lastFailure), + mockCtx, + action, + startTime, + kind, + ); + const policyTask = new RetryableTask( + new RetryPolicy({ + firstRetryIntervalInMilliseconds: 1, + maxNumberOfAttempts: 3, + handleFailure: inspect, + }), + action, + startTime, + kind, + ); + const failure = new pb.TaskFailureDetails().setInnerfailure( + new pb.TaskFailureDetails().setIsnonretriable(true), + ); + handlerTask.recordFailure("", failure); + policyTask.recordFailure("", failure); + expect(await handlerTask.shouldRetry(startTime)).toBe(true); + expect(policyTask.computeNextDelayInMilliseconds(startTime)).toBe(1); + expect(inspect).toHaveBeenCalledTimes(2); + failure.setIsnonretriable(true); + expect(await handlerTask.shouldRetry(startTime)).toBe(false); + expect(policyTask.computeNextDelayInMilliseconds(startTime)).toBeUndefined(); + expect(inspect).toHaveBeenCalledTimes(2); + }, + ); + it("should return false when no failure recorded", async () => { // Arrange const handler: AsyncRetryHandler = async () => true; From 6f0aeae056a7db02f3fe5940e75510b4df19a4c7 Mon Sep 17 00:00:00 2001 From: wangbill Date: Thu, 24 Sep 2026 13:37:09 -0700 Subject: [PATCH 2/2] fix: stop circular failure detail forwarding Track received failure objects by identity and append a CircularFailureDetails marker on a repeated reference. Preserve the task wrapper and unique prefix without truncating finite chains or changing ordinary Error.cause handling. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e0af01a5-0dfa-4e71-a660-c4186e65d7e0 --- CHANGELOG.md | 2 + README.md | 3 + .../src/utils/pb-helper.util.ts | 10 +++ .../test/failure-details.spec.ts | 76 ++++++++++++++++++- 4 files changed, 88 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9ebdfe4..6c4251e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,8 @@ client state/history, and in-memory testing. Uncaught or rethrown task failures retain their received chain beneath the existing `TaskFailedError` wrapper. Retry decisions and the ordinary `Error.cause` serialization depth limit are unchanged. + User-mutated circular detail chains terminate with an explicit `CircularFailureDetails` + marker after the unique prefix, without imposing a depth limit on finite received chains. - Preserve explicit empty activity and child versions on the protobuf wire, including the unversioned child default, so concrete empty-version work-item filters can match them. - Reject uninitialized `whenAll` results after a canceled child's completion callback throws, diff --git a/README.md b/README.md index 8373fc53..943a6575 100644 --- a/README.md +++ b/README.md @@ -219,6 +219,9 @@ Missing inner failures remain `undefined`; existing two- and three-argument details. Remote failures are not reconstructed as JavaScript `Error.cause` objects. The existing writer still limits ordinary `Error.cause` chains to ten inner levels; reading or forwarding already-received failure details does not add a truncation limit. +If user code mutates received details into a cycle, forwarding preserves each unique +failure and ends the chain with `errorType: "CircularFailureDetails"` and +`message: "A circular innerFailure reference was detected."` rather than looping. ### Reusing orchestration instance IDs diff --git a/packages/durabletask-js/src/utils/pb-helper.util.ts b/packages/durabletask-js/src/utils/pb-helper.util.ts index d273f042..34721eb2 100644 --- a/packages/durabletask-js/src/utils/pb-helper.util.ts +++ b/packages/durabletask-js/src/utils/pb-helper.util.ts @@ -241,7 +241,17 @@ function buildFailureDetails(e: unknown, depth: number): pb.TaskFailureDetails { if (e instanceof TaskFailedError && e.cause == null) { // Keep the task wrapper and attach the received failure chain without rebuilding Error objects. let current = failure; + const seen = new Set(); for (let details: TaskFailureDetails | undefined = e.details; details; details = details.innerFailure) { + if (seen.has(details)) { + current.setInnerfailure( + new pb.TaskFailureDetails() + .setErrortype("CircularFailureDetails") + .setErrormessage("A circular innerFailure reference was detected."), + ); + break; + } + seen.add(details); const inner = new pb.TaskFailureDetails(); inner.setErrortype(details.errorType); inner.setErrormessage(details.message); diff --git a/packages/durabletask-js/test/failure-details.spec.ts b/packages/durabletask-js/test/failure-details.spec.ts index ad3bd053..276a8f89 100644 --- a/packages/durabletask-js/test/failure-details.spec.ts +++ b/packages/durabletask-js/test/failure-details.spec.ts @@ -30,6 +30,17 @@ function errorChain(): Error { return outer; } +function addBoundedCycle(tail: TaskFailureDetails, target: TaskFailureDetails) { + let reads = 0; + const get = jest.fn(() => { + // Keep a regressed serializer from hanging the test process. + if (++reads > 4) throw new Error("Repeatedly traversed a circular innerFailure"); + return target; + }); + Object.defineProperty(tail, "innerFailure", { get }); + return get; +} + const expectedChain = { errorType: "OrderFailed", message: "Order failed", @@ -96,8 +107,47 @@ describe("Public failure details", () => { it("retains explicitly assigned JavaScript causes on task errors", () => { const error = new TaskFailedError("failed", newFailureDetails(errorChain())); + const getInnerFailure = addBoundedCycle(error.details, error.details); error.cause = new Error("explicit cause"); expect(newFailureDetails(error).getInnerfailure()?.getErrormessage()).toBe("explicit cause"); + expect(getInnerFailure).not.toHaveBeenCalled(); + }); + + it.each([1, 3])("marks a %i-node cycle after preserving each unique failure and the task wrapper", (length) => { + const proto = newFailureDetails(errorChain()); + const error = new TaskFailedError("Activity task failed", proto); + let tail: TaskFailureDetails = error.details; + for (let i = 1; i < length; i++) tail = tail.innerFailure!; + const getInnerFailure = addBoundedCycle(tail, error.details); + + const forwarded = newFailureDetails(error); + expect(forwarded.getErrortype()).toBe("TaskFailedError"); + expect(forwarded.getErrormessage()).toBe(error.message); + expect(forwarded.getStacktrace()?.getValue()).toBe(error.stack); + let actual = forwarded.getInnerfailure(); + let expected: pb.TaskFailureDetails | undefined = proto; + for (let i = 0; i < length; i++) { + expect(actual?.getErrortype()).toBe(expected?.getErrortype()); + expect(actual?.getErrormessage()).toBe(expected?.getErrormessage()); + expect(actual?.getStacktrace()?.getValue()).toBe(expected?.getStacktrace()?.getValue()); + actual = actual?.getInnerfailure(); + expected = expected?.getInnerfailure(); + } + expect(actual?.getErrortype()).toBe("CircularFailureDetails"); + expect(actual?.getErrormessage()).toBe("A circular innerFailure reference was detected."); + expect(actual?.getStacktrace()).toBeUndefined(); + expect(actual?.getInnerfailure()).toBeUndefined(); + expect(getInnerFailure).toHaveBeenCalledTimes(1); + }); + + it("does not treat distinct failure objects with identical fields as a cycle", () => { + const proto = new pb.TaskFailureDetails() + .setErrortype("Error") + .setErrormessage("same") + .setInnerfailure(new pb.TaskFailureDetails().setErrortype("Error").setErrormessage("same")); + const error = new TaskFailedError("failed", proto); + expect(error.details.innerFailure).not.toBe(error.details); + expect(newFailureDetails(error).getInnerfailure()?.serializeBinary()).toEqual(proto.serializeBinary()); }); it("preserves the chain in orchestration state and raiseIfFailed", () => { @@ -208,7 +258,7 @@ describe.each(["activity", "subOrchestration"] as const)("In-memory %s failure c ); }); -it("preserves an uncaught activity cause chain in the terminal client failure", async () => { +it.each([false, true])("preserves terminal activity failure details with a cyclic rethrow: %s", async (cyclic) => { const backend = new InMemoryOrchestrationBackend(); const client = new TestOrchestrationClient(backend); const worker = new TestOrchestrationWorker(backend); @@ -216,16 +266,36 @@ it("preserves an uncaught activity cause chain in the terminal client failure", throw errorChain(); }); worker.addNamedOrchestrator("order", async function* (ctx) { - yield ctx.callActivity("fail"); + if (!cyclic) { + yield ctx.callActivity("fail"); + } else { + try { + yield ctx.callActivity("fail"); + } catch (error) { + if (!(error instanceof TaskFailedError)) throw error; + addBoundedCycle(error.details, error.details); + throw error; + } + } }); await worker.start(); try { const id = await client.scheduleNewOrchestration("order"); const state = await client.waitForOrchestrationCompletion(id, true, 5); + expect(state?.runtimeStatus).toBe(OrchestrationStatus.FAILED); expect(state?.failureDetails).toMatchObject({ errorType: "TaskFailedError", message: "Activity task #1 failed: Order failed", - innerFailure: expectedChain, + innerFailure: cyclic + ? { + ...expectedChain, + innerFailure: { + errorType: "CircularFailureDetails", + message: "A circular innerFailure reference was detected.", + innerFailure: undefined, + }, + } + : expectedChain, }); } finally { await worker.stop();