From 47f58de110f19cd01011670e6b1e6170ed2028bb Mon Sep 17 00:00:00 2001 From: Copilot Date: Wed, 18 Mar 2026 08:34:03 +0000 Subject: [PATCH] [copilot-finds] Bug: newOrchestrationState drops failure details when error message or type is empty string The newOrchestrationState function in orchestration/index.ts used falsy string checks (failureDetailsErrorMessage && failureDetailsErrorType) to determine whether to create FailureDetails. Empty strings are falsy in JavaScript, so failure details with an empty error message or empty error type were silently dropped, returning undefined failureDetails even for failed orchestrations. This fix changes the check to verify the protobuf TaskFailureDetails object exists (consistent with the pattern used in client.ts and history-event-converter.ts). Also fixes getStacktrace()?.toString() to use the canonical .getValue() method used everywhere else in the codebase. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- package-lock.json | 17 -- .../durabletask-js/src/orchestration/index.ts | 12 +- .../test/new-orchestration-state.spec.ts | 223 ++++++++++++++++++ 3 files changed, 228 insertions(+), 24 deletions(-) create mode 100644 packages/durabletask-js/test/new-orchestration-state.spec.ts diff --git a/package-lock.json b/package-lock.json index d67bef9d..4757e912 100644 --- a/package-lock.json +++ b/package-lock.json @@ -69,7 +69,6 @@ "resolved": "https://registry.npmjs.org/@azure/core-client/-/core-client-1.10.1.tgz", "integrity": "sha512-Nh5PhEOeY6PrnxNPsEHRr9eimxLwgLlpmguQaHKBinFYA/RU9+kOYVOQqOrTsCL+KSxrLLl1gD8Dk5BFW/7l/w==", "license": "MIT", - "peer": true, "dependencies": { "@azure/abort-controller": "^2.1.2", "@azure/core-auth": "^1.10.0", @@ -131,7 +130,6 @@ "resolved": "https://registry.npmjs.org/@azure/core-rest-pipeline/-/core-rest-pipeline-1.22.2.tgz", "integrity": "sha512-MzHym+wOi8CLUlKCQu12de0nwcq9k9Kuv43j4Wa++CsCpJwps2eeBQwD2Bu8snkxTtDKDx4GwjuR9E8yC8LNrg==", "license": "MIT", - "peer": true, "dependencies": { "@azure/abort-controller": "^2.1.2", "@azure/core-auth": "^1.10.0", @@ -330,7 +328,6 @@ "integrity": "sha512-H3mcG6ZDLTlYfaSNi0iOKkigqMFvkTKlGUYlD8GW7nNOYRrevuA46iTypPyv+06V3fEmvvazfntkBU34L0azAw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@babel/code-frame": "^7.28.6", "@babel/generator": "^7.28.6", @@ -1002,7 +999,6 @@ "resolved": "https://registry.npmjs.org/@grpc/grpc-js/-/grpc-js-1.14.3.tgz", "integrity": "sha512-Iq8QQQ/7X3Sac15oB6p0FmUg/klxQvXLeileoqrTRGJYLV+/9tubbr9ipz0GKHjmXVsgFPo/+W+2cA8eNcR+XA==", "license": "Apache-2.0", - "peer": true, "dependencies": { "@grpc/proto-loader": "^0.8.0", "@js-sdsl/ordered-map": "^4.4.2" @@ -1603,7 +1599,6 @@ "integrity": "sha512-3giAOQvZiH5F9bMlMiv8+GSPMeqg0dbaeo58/0SlA9sxSqZhnUtxzX9/2FzyhS9sWQf5S0GJE0AKBrFqjpeYcg==", "dev": true, "license": "Apache-2.0", - "peer": true, "engines": { "node": ">=8.0.0" } @@ -1780,7 +1775,6 @@ "dev": true, "hasInstallScript": true, "license": "Apache-2.0", - "peer": true, "dependencies": { "@swc/counter": "^0.1.3", "@swc/types": "^0.1.25" @@ -1996,7 +1990,6 @@ "integrity": "sha512-TXTnIcNJQEKwThMMqBXsZ4VGAza6bvN4pa41Rkqoio6QBKMvo+5lexeTMScGCIxtzgQJzElcvIltani+adC5PQ==", "dev": true, "license": "Apache-2.0", - "peer": true, "dependencies": { "tslib": "^2.8.0" } @@ -2158,7 +2151,6 @@ "resolved": "https://registry.npmjs.org/@types/node/-/node-22.19.9.tgz", "integrity": "sha512-PD03/U8g1F9T9MI+1OBisaIARhSzeidsUjQaf51fOxrfjeiKN9bLVO06lHuHYjxdnqLWJijJHfqXPSJri2EM2A==", "license": "MIT", - "peer": true, "dependencies": { "undici-types": "~6.21.0" } @@ -2232,7 +2224,6 @@ "integrity": "sha512-BtE0k6cjwjLZoZixN0t5AKP0kSzlGu7FctRXYuPAm//aaiZhmfq1JwdYpYr1brzEspYyFeF+8XF5j2VK6oalrA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.54.0", "@typescript-eslint/types": "8.54.0", @@ -2474,7 +2465,6 @@ "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "dev": true, "license": "MIT", - "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -2799,7 +2789,6 @@ } ], "license": "MIT", - "peer": true, "dependencies": { "baseline-browser-mapping": "^2.9.0", "caniuse-lite": "^1.0.30001759", @@ -3447,7 +3436,6 @@ "integrity": "sha512-LEyamqS7W5HB3ujJyvi0HQK/dtVINZvd5mAAp9eT5S/ujByGjiZLCzPcHVzuXbpJDJF/cxwHlfceVUDZ2lnSTw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.8.0", "@eslint-community/regexpp": "^4.12.1", @@ -4407,7 +4395,6 @@ "integrity": "sha512-NIy3oAFp9shda19hy4HK0HRTWKtPJmGdnvywu01nOqNC2vZg+Z+fvJDxpMQA88eb2I9EcafcdjYgsDthnYTvGw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@jest/core": "^29.7.0", "@jest/types": "^29.6.3", @@ -6169,7 +6156,6 @@ "integrity": "sha512-UOnG6LftzbdaHZcKoPFtOcCKztrQ57WkHDeRD9t/PTQtmT0NHSeWWepj6pS0z/N7+08BHFDQVUrfmfMRcZwbMg==", "dev": true, "license": "MIT", - "peer": true, "bin": { "prettier": "bin/prettier.cjs" }, @@ -6906,7 +6892,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -7058,7 +7043,6 @@ "integrity": "sha512-f0FFpIdcHgn8zcPSbf1dRevwt047YMnaiJM3u2w2RewrB+fob/zePZcrOyQoLMMO7aBIddLcQIEK5dYjkLnGrQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@cspotcode/source-map-support": "^0.8.0", "@tsconfig/node10": "^1.0.7", @@ -7145,7 +7129,6 @@ "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", "dev": true, "license": "Apache-2.0", - "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" diff --git a/packages/durabletask-js/src/orchestration/index.ts b/packages/durabletask-js/src/orchestration/index.ts index 74cea611..ebec557b 100644 --- a/packages/durabletask-js/src/orchestration/index.ts +++ b/packages/durabletask-js/src/orchestration/index.ts @@ -18,14 +18,12 @@ export function newOrchestrationState( const state = res.getOrchestrationstate(); let failureDetails; - const failureDetailsErrorMessage = state?.getFailuredetails()?.getErrormessage(); - const failureDetailsErrorType = state?.getFailuredetails()?.getErrortype(); - - if (state && failureDetailsErrorMessage && failureDetailsErrorType) { + const protoFailureDetails = state?.getFailuredetails(); + if (protoFailureDetails) { failureDetails = new FailureDetails( - failureDetailsErrorMessage, - failureDetailsErrorType, - state.getFailuredetails()?.getStacktrace()?.toString(), + protoFailureDetails.getErrormessage(), + protoFailureDetails.getErrortype(), + protoFailureDetails.getStacktrace()?.getValue(), ); } diff --git a/packages/durabletask-js/test/new-orchestration-state.spec.ts b/packages/durabletask-js/test/new-orchestration-state.spec.ts new file mode 100644 index 00000000..da7fbbc0 --- /dev/null +++ b/packages/durabletask-js/test/new-orchestration-state.spec.ts @@ -0,0 +1,223 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as pb from "../src/proto/orchestrator_service_pb"; +import { StringValue } from "google-protobuf/google/protobuf/wrappers_pb"; +import { Timestamp } from "google-protobuf/google/protobuf/timestamp_pb"; +import { newOrchestrationState } from "../src/orchestration"; + +/** + * Helper to create a GetInstanceResponse with the given orchestration state. + */ +function createResponse( + name: string, + status: pb.OrchestrationStatus, + options?: { + failureDetails?: pb.TaskFailureDetails; + input?: string; + output?: string; + customStatus?: string; + }, +): pb.GetInstanceResponse { + const state = new pb.OrchestrationState(); + state.setName(name); + state.setOrchestrationstatus(status); + + const now = new Timestamp(); + now.fromDate(new Date()); + state.setCreatedtimestamp(now); + state.setLastupdatedtimestamp(now); + + if (options?.failureDetails) { + state.setFailuredetails(options.failureDetails); + } + + if (options?.input !== undefined) { + const inputValue = new StringValue(); + inputValue.setValue(options.input); + state.setInput(inputValue); + } + + if (options?.output !== undefined) { + const outputValue = new StringValue(); + outputValue.setValue(options.output); + state.setOutput(outputValue); + } + + if (options?.customStatus !== undefined) { + const customStatusValue = new StringValue(); + customStatusValue.setValue(options.customStatus); + state.setCustomstatus(customStatusValue); + } + + const res = new pb.GetInstanceResponse(); + res.setExists(true); + res.setOrchestrationstate(state); + return res; +} + +describe("newOrchestrationState", () => { + it("should return undefined when response is undefined", () => { + const result = newOrchestrationState("test-id", undefined); + expect(result).toBeUndefined(); + }); + + it("should return undefined when instance does not exist", () => { + const res = new pb.GetInstanceResponse(); + res.setExists(false); + const result = newOrchestrationState("test-id", res); + expect(result).toBeUndefined(); + }); + + it("should return state for a completed orchestration", () => { + const res = createResponse( + "TestOrchestrator", + pb.OrchestrationStatus.ORCHESTRATION_STATUS_COMPLETED, + { output: '"hello"' }, + ); + + const result = newOrchestrationState("test-id", res); + + expect(result).toBeDefined(); + expect(result!.instanceId).toBe("test-id"); + expect(result!.name).toBe("TestOrchestrator"); + expect(result!.serializedOutput).toBe('"hello"'); + expect(result!.failureDetails).toBeUndefined(); + }); + + it("should preserve failure details with non-empty error message", () => { + const failureDetails = new pb.TaskFailureDetails(); + failureDetails.setErrormessage("Something went wrong"); + failureDetails.setErrortype("Error"); + const stackValue = new StringValue(); + stackValue.setValue("Error: Something went wrong\n at test.ts:1"); + failureDetails.setStacktrace(stackValue); + + const res = createResponse( + "TestOrchestrator", + pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED, + { failureDetails }, + ); + + const result = newOrchestrationState("test-id", res); + + expect(result).toBeDefined(); + expect(result!.failureDetails).toBeDefined(); + expect(result!.failureDetails!.message).toBe("Something went wrong"); + expect(result!.failureDetails!.errorType).toBe("Error"); + expect(result!.failureDetails!.stackTrace).toBe( + "Error: Something went wrong\n at test.ts:1", + ); + }); + + it("should preserve failure details when error message is an empty string", () => { + const failureDetails = new pb.TaskFailureDetails(); + failureDetails.setErrormessage(""); + failureDetails.setErrortype("Error"); + + const res = createResponse( + "TestOrchestrator", + pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED, + { failureDetails }, + ); + + const result = newOrchestrationState("test-id", res); + + expect(result).toBeDefined(); + expect(result!.failureDetails).toBeDefined(); + expect(result!.failureDetails!.message).toBe(""); + expect(result!.failureDetails!.errorType).toBe("Error"); + }); + + it("should preserve failure details when error type is an empty string", () => { + const failureDetails = new pb.TaskFailureDetails(); + failureDetails.setErrormessage("Some error"); + failureDetails.setErrortype(""); + + const res = createResponse( + "TestOrchestrator", + pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED, + { failureDetails }, + ); + + const result = newOrchestrationState("test-id", res); + + expect(result).toBeDefined(); + expect(result!.failureDetails).toBeDefined(); + expect(result!.failureDetails!.message).toBe("Some error"); + expect(result!.failureDetails!.errorType).toBe(""); + }); + + it("should preserve failure details when both error message and type are empty strings", () => { + const failureDetails = new pb.TaskFailureDetails(); + failureDetails.setErrormessage(""); + failureDetails.setErrortype(""); + + const res = createResponse( + "TestOrchestrator", + pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED, + { failureDetails }, + ); + + const result = newOrchestrationState("test-id", res); + + expect(result).toBeDefined(); + expect(result!.failureDetails).toBeDefined(); + expect(result!.failureDetails!.message).toBe(""); + expect(result!.failureDetails!.errorType).toBe(""); + }); + + it("should not set failure details when no failure details exist in response", () => { + const res = createResponse( + "TestOrchestrator", + pb.OrchestrationStatus.ORCHESTRATION_STATUS_COMPLETED, + ); + + const result = newOrchestrationState("test-id", res); + + expect(result).toBeDefined(); + expect(result!.failureDetails).toBeUndefined(); + }); + + it("should extract stack trace using getValue() instead of toString()", () => { + const failureDetails = new pb.TaskFailureDetails(); + failureDetails.setErrormessage("test error"); + failureDetails.setErrortype("TypeError"); + const stackValue = new StringValue(); + stackValue.setValue("TypeError: test error\n at Object. (test.ts:1:1)"); + failureDetails.setStacktrace(stackValue); + + const res = createResponse( + "TestOrchestrator", + pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED, + { failureDetails }, + ); + + const result = newOrchestrationState("test-id", res); + + expect(result).toBeDefined(); + expect(result!.failureDetails).toBeDefined(); + expect(result!.failureDetails!.stackTrace).toBe( + "TypeError: test error\n at Object. (test.ts:1:1)", + ); + }); + + it("should handle failure details without stack trace", () => { + const failureDetails = new pb.TaskFailureDetails(); + failureDetails.setErrormessage("error without stack"); + failureDetails.setErrortype("Error"); + + const res = createResponse( + "TestOrchestrator", + pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED, + { failureDetails }, + ); + + const result = newOrchestrationState("test-id", res); + + expect(result).toBeDefined(); + expect(result!.failureDetails).toBeDefined(); + expect(result!.failureDetails!.message).toBe("error without stack"); + expect(result!.failureDetails!.stackTrace).toBeUndefined(); + }); +});