From 52a2938ffbaac3e7978b844856b4e6dbc4d364ae Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Sat, 11 Apr 2026 08:36:39 +0000 Subject: [PATCH 1/2] fix: read span error message from failureDetails for failed orchestrations setOrchestrationStatusFromActions() was reading the error message from completeAction.getResult(), but failed orchestrations store the error in failureDetails (not result). This caused all failed orchestration spans to show the generic 'Orchestration failed' message instead of the actual error. Now reads from failureDetails.getErrormessage() first, with fallback to result for backwards compatibility. Fixes #218 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/tracing/trace-helper.ts | 9 +++- packages/durabletask-js/test/tracing.spec.ts | 50 +++++++++++++++++-- 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/packages/durabletask-js/src/tracing/trace-helper.ts b/packages/durabletask-js/src/tracing/trace-helper.ts index aecbb46..01e596a 100644 --- a/packages/durabletask-js/src/tracing/trace-helper.ts +++ b/packages/durabletask-js/src/tracing/trace-helper.ts @@ -690,9 +690,14 @@ export function setOrchestrationStatusFromActions( span.setAttribute(DurableTaskAttributes.TASK_STATUS, statusName); } - // Match .NET: set span error status when orchestration completes with Failed + // Match .NET: set span error status when orchestration completes with Failed. + // Read error message from failureDetails (where setFailed() stores it), + // falling back to result for backwards compatibility. if (status === pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED) { - const errorMessage = completeAction.getResult()?.getValue() ?? "Orchestration failed"; + const errorMessage = + completeAction.getFailuredetails()?.getErrormessage() || + completeAction.getResult()?.getValue() || + "Orchestration failed"; span.setStatus({ code: otel.SpanStatusCode.ERROR, message: errorMessage }); } else { span.setStatus({ code: otel.SpanStatusCode.OK }); diff --git a/packages/durabletask-js/test/tracing.spec.ts b/packages/durabletask-js/test/tracing.spec.ts index 4e2dfa0..c5d447f 100644 --- a/packages/durabletask-js/test/tracing.spec.ts +++ b/packages/durabletask-js/test/tracing.spec.ts @@ -1124,15 +1124,17 @@ describe("Trace Helper - setOrchestrationStatusFromActions", () => { expect(spans[0].status.code).toBe(otel.SpanStatusCode.OK); }); - it("should set status attribute for failed orchestration and ERROR span status", () => { + it("should set status attribute for failed orchestration and ERROR span status with failureDetails", () => { const tracer = otel.trace.getTracer(TRACER_NAME); const span = tracer.startSpan("orch-status-failed"); + // Match the real runtime path: setFailed() sets failureDetails but NOT result const completeAction = new pb.CompleteOrchestrationAction(); completeAction.setOrchestrationstatus(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED); - const resultValue = new StringValue(); - resultValue.setValue("User code threw an error"); - completeAction.setResult(resultValue); + const failureDetails = new pb.TaskFailureDetails(); + failureDetails.setErrormessage("User code threw an error"); + failureDetails.setErrortype("Error"); + completeAction.setFailuredetails(failureDetails); const action = new pb.OrchestratorAction(); action.setCompleteorchestration(completeAction); @@ -1146,6 +1148,46 @@ describe("Trace Helper - setOrchestrationStatusFromActions", () => { expect(spans[0].status.message).toBe("User code threw an error"); }); + it("should fall back to result field for failed orchestration error message", () => { + const tracer = otel.trace.getTracer(TRACER_NAME); + const span = tracer.startSpan("orch-status-failed-result-fallback"); + + // Edge case: no failureDetails, but result is set (backwards compatibility) + const completeAction = new pb.CompleteOrchestrationAction(); + completeAction.setOrchestrationstatus(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED); + const resultValue = new StringValue(); + resultValue.setValue("Legacy error message"); + completeAction.setResult(resultValue); + + const action = new pb.OrchestratorAction(); + action.setCompleteorchestration(completeAction); + + setOrchestrationStatusFromActions(span, [action]); + span.end(); + + const spans = exporter.getFinishedSpans(); + expect(spans[0].status.code).toBe(otel.SpanStatusCode.ERROR); + expect(spans[0].status.message).toBe("Legacy error message"); + }); + + it("should use default error message when neither failureDetails nor result is set", () => { + const tracer = otel.trace.getTracer(TRACER_NAME); + const span = tracer.startSpan("orch-status-failed-no-details"); + + const completeAction = new pb.CompleteOrchestrationAction(); + completeAction.setOrchestrationstatus(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED); + + const action = new pb.OrchestratorAction(); + action.setCompleteorchestration(completeAction); + + setOrchestrationStatusFromActions(span, [action]); + span.end(); + + const spans = exporter.getFinishedSpans(); + expect(spans[0].status.code).toBe(otel.SpanStatusCode.ERROR); + expect(spans[0].status.message).toBe("Orchestration failed"); + }); + it("should not set status when no completion action present", () => { const tracer = otel.trace.getTracer(TRACER_NAME); const span = tracer.startSpan("orch-status-none"); From 058485fe4f3d09eeb953c11e437195bd5a578d25 Mon Sep 17 00:00:00 2001 From: wangbill Date: Mon, 13 Jul 2026 15:45:48 -0700 Subject: [PATCH 2/2] Use nullish coalescing for failed orchestration span message Match the task/sub-orchestration failure paths and .NET's TraceHelper by reading the failed-orchestration span message with ?? instead of ||, so an explicitly empty failureDetails error message is preserved as the authoritative message rather than masked by the result/default fallback. Add a regression test covering the empty-message case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/tracing/trace-helper.ts | 9 ++++-- packages/durabletask-js/test/tracing.spec.ts | 28 +++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/packages/durabletask-js/src/tracing/trace-helper.ts b/packages/durabletask-js/src/tracing/trace-helper.ts index 01e596a..8fcf3cf 100644 --- a/packages/durabletask-js/src/tracing/trace-helper.ts +++ b/packages/durabletask-js/src/tracing/trace-helper.ts @@ -692,11 +692,14 @@ export function setOrchestrationStatusFromActions( // Match .NET: set span error status when orchestration completes with Failed. // Read error message from failureDetails (where setFailed() stores it), - // falling back to result for backwards compatibility. + // falling back to result for backwards compatibility. Use nullish coalescing + // (like the task/sub-orchestration failure paths above and .NET's TraceHelper) + // so an explicitly empty failureDetails message is preserved rather than + // masked by the generic fallback. if (status === pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED) { const errorMessage = - completeAction.getFailuredetails()?.getErrormessage() || - completeAction.getResult()?.getValue() || + completeAction.getFailuredetails()?.getErrormessage() ?? + completeAction.getResult()?.getValue() ?? "Orchestration failed"; span.setStatus({ code: otel.SpanStatusCode.ERROR, message: errorMessage }); } else { diff --git a/packages/durabletask-js/test/tracing.spec.ts b/packages/durabletask-js/test/tracing.spec.ts index c5d447f..cab722f 100644 --- a/packages/durabletask-js/test/tracing.spec.ts +++ b/packages/durabletask-js/test/tracing.spec.ts @@ -1188,6 +1188,34 @@ describe("Trace Helper - setOrchestrationStatusFromActions", () => { expect(spans[0].status.message).toBe("Orchestration failed"); }); + it("should preserve an explicitly empty failureDetails message over the result/default fallback", () => { + const tracer = otel.trace.getTracer(TRACER_NAME); + const span = tracer.startSpan("orch-status-failed-empty-message"); + + // failureDetails is present (orchestration failed) but the error message is + // empty. failureDetails is authoritative, so nullish coalescing must keep the + // empty string rather than falling through to result/"Orchestration failed". + const completeAction = new pb.CompleteOrchestrationAction(); + completeAction.setOrchestrationstatus(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED); + const failureDetails = new pb.TaskFailureDetails(); + failureDetails.setErrormessage(""); + failureDetails.setErrortype("Error"); + completeAction.setFailuredetails(failureDetails); + const resultValue = new StringValue(); + resultValue.setValue("Should not be used"); + completeAction.setResult(resultValue); + + const action = new pb.OrchestratorAction(); + action.setCompleteorchestration(completeAction); + + setOrchestrationStatusFromActions(span, [action]); + span.end(); + + const spans = exporter.getFinishedSpans(); + expect(spans[0].status.code).toBe(otel.SpanStatusCode.ERROR); + expect(spans[0].status.message).toBe(""); + }); + it("should not set status when no completion action present", () => { const tracer = otel.trace.getTracer(TRACER_NAME); const span = tracer.startSpan("orch-status-none");