From d7f32d936d852b5368df7786412f11c1eb116611 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Mon, 20 Apr 2026 09:15:02 +0000 Subject: [PATCH] [copilot-finds] Bug: handleEntityOperationFailed discards EntityOperationFailedException, throwing generic TaskFailedError instead The handleEntityOperationFailed method in orchestration-executor.ts creates an EntityOperationFailedException but immediately discards it, only extracting its .message property. The actual error thrown to user code is always a generic TaskFailedError, violating the documented API contract that callEntity() throws EntityOperationFailedException. Changes: - Make EntityOperationFailedException extend TaskFailedError for backward compat - Add failWithError() to CompletableTask for pre-constructed exceptions - Update handleEntityOperationFailed to use the EntityOperationFailedException - Add tests verifying correct exception type and entity-specific context Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../entity-operation-failed-exception.ts | 39 +++++++++++- .../src/task/completable-task.ts | 18 ++++++ .../src/worker/orchestration-executor.ts | 18 ++++-- .../test/entity-operation-events.spec.ts | 59 ++++++++++++++++++- .../entity-operation-failed-exception.spec.ts | 19 ++++++ 5 files changed, 144 insertions(+), 9 deletions(-) diff --git a/packages/durabletask-js/src/entities/entity-operation-failed-exception.ts b/packages/durabletask-js/src/entities/entity-operation-failed-exception.ts index 35c7308..ffd557d 100644 --- a/packages/durabletask-js/src/entities/entity-operation-failed-exception.ts +++ b/packages/durabletask-js/src/entities/entity-operation-failed-exception.ts @@ -3,6 +3,8 @@ import { EntityInstanceId } from "./entity-instance-id"; import * as pb from "../proto/orchestrator_service_pb"; +import { StringValue } from "google-protobuf/google/protobuf/wrappers_pb"; +import { TaskFailedError } from "../task/exception/task-failed-error"; /** * Details about a task failure. @@ -52,14 +54,36 @@ export function createTaskFailureDetails(proto: pb.TaskFailureDetails | undefine }; } +/** + * Converts an entity TaskFailureDetails interface to a protobuf TaskFailureDetails message. + */ +function toProtoFailureDetails(details: TaskFailureDetails): pb.TaskFailureDetails { + const proto = new pb.TaskFailureDetails(); + proto.setErrortype(details.errorType); + proto.setErrormessage(details.errorMessage); + if (details.stackTrace) { + const sv = new StringValue(); + sv.setValue(details.stackTrace); + proto.setStacktrace(sv); + } + if (details.innerFailure) { + proto.setInnerfailure(toProtoFailureDetails(details.innerFailure)); + } + return proto; +} + /** * Exception that gets thrown when an entity operation fails with an unhandled exception. * * @remarks + * Extends `TaskFailedError` so that existing `catch` blocks checking + * `instanceof TaskFailedError` continue to work, while also allowing + * more specific `instanceof EntityOperationFailedException` checks. + * * Detailed information associated with a particular operation failure, including exception details, * can be found in the `failureDetails` property. */ -export class EntityOperationFailedException extends Error { +export class EntityOperationFailedException extends TaskFailedError { /** * The ID of the entity. */ @@ -81,9 +105,18 @@ export class EntityOperationFailedException extends Error { * @param entityId - The entity ID. * @param operationName - The operation name. * @param failureDetails - The failure details. + * @param protoFailureDetails - Optional protobuf failure details. If not provided, + * they are constructed from `failureDetails`. */ - constructor(entityId: EntityInstanceId, operationName: string, failureDetails: TaskFailureDetails) { - super(EntityOperationFailedException.getExceptionMessage(operationName, entityId, failureDetails)); + constructor( + entityId: EntityInstanceId, + operationName: string, + failureDetails: TaskFailureDetails, + protoFailureDetails?: pb.TaskFailureDetails, + ) { + const message = EntityOperationFailedException.getExceptionMessage(operationName, entityId, failureDetails); + const protoDetails = protoFailureDetails ?? toProtoFailureDetails(failureDetails); + super(message, protoDetails); this.name = "EntityOperationFailedException"; this.entityId = entityId; this.operationName = operationName; diff --git a/packages/durabletask-js/src/task/completable-task.ts b/packages/durabletask-js/src/task/completable-task.ts index 2171a32..4ddfb11 100644 --- a/packages/durabletask-js/src/task/completable-task.ts +++ b/packages/durabletask-js/src/task/completable-task.ts @@ -37,4 +37,22 @@ export class CompletableTask extends Task { this._parent.onChildCompleted(this); } } + + /** + * Fails the task with a pre-constructed error. + * Use this when a more specific error subclass (e.g., EntityOperationFailedException) + * should be preserved as the task's exception rather than wrapping in a generic TaskFailedError. + */ + failWithError(error: TaskFailedError): void { + if (this._isComplete) { + throw new Error("Task is already completed"); + } + + this._exception = error; + this._isComplete = true; + + if (this._parent) { + this._parent.onChildCompleted(this); + } + } } diff --git a/packages/durabletask-js/src/worker/orchestration-executor.ts b/packages/durabletask-js/src/worker/orchestration-executor.ts index e081d32..d5f6d60 100644 --- a/packages/durabletask-js/src/worker/orchestration-executor.ts +++ b/packages/durabletask-js/src/worker/orchestration-executor.ts @@ -28,6 +28,7 @@ import { RuntimeOrchestrationContext } from "./runtime-orchestration-context"; import { EntityOperationFailedException, createTaskFailureDetails, + TaskFailureDetails as EntityTaskFailureDetails, } from "../entities/entity-operation-failed-exception"; /** @@ -598,19 +599,28 @@ export class OrchestrationExecutor { // If in a critical section, recover the lock for this entity ctx._entityFeature.recoverLockAfterCall(pendingCall.entityId); - // Convert failure details and throw EntityOperationFailedException + // Convert failure details and fail the task with EntityOperationFailedException const failureDetails = createTaskFailureDetails(failedEvent?.getFailuredetails()); if (!failureDetails) { - pendingCall.task.fail( - `Entity operation '${pendingCall.operationName}' failed with unknown error`, + const unknownFailure: EntityTaskFailureDetails = { + errorType: "UnknownError", + errorMessage: `Entity operation '${pendingCall.operationName}' failed with unknown error`, + }; + const exception = new EntityOperationFailedException( + pendingCall.entityId, + pendingCall.operationName, + unknownFailure, + failedEvent?.getFailuredetails(), ); + pendingCall.task.failWithError(exception); } else { const exception = new EntityOperationFailedException( pendingCall.entityId, pendingCall.operationName, failureDetails, + failedEvent?.getFailuredetails(), ); - pendingCall.task.fail(exception.message, failedEvent?.getFailuredetails()); + pendingCall.task.failWithError(exception); } await ctx.resume(); diff --git a/packages/durabletask-js/test/entity-operation-events.spec.ts b/packages/durabletask-js/test/entity-operation-events.spec.ts index 2766dd0..e3ed8b5 100644 --- a/packages/durabletask-js/test/entity-operation-events.spec.ts +++ b/packages/durabletask-js/test/entity-operation-events.spec.ts @@ -5,6 +5,8 @@ import { OrchestrationExecutor } from "../src/worker/orchestration-executor"; import { Registry } from "../src/worker/registry"; import { OrchestrationContext } from "../src/task/context/orchestration-context"; import { EntityInstanceId } from "../src/entities/entity-instance-id"; +import { EntityOperationFailedException } from "../src/entities/entity-operation-failed-exception"; +import { TaskFailedError } from "../src/task/exception/task-failed-error"; import * as pb from "../src/proto/orchestrator_service_pb"; import * as ph from "../src/utils/pb-helper.util"; import { StringValue } from "google-protobuf/google/protobuf/wrappers_pb"; @@ -190,7 +192,7 @@ describe("OrchestrationExecutor Entity Operation Events", () => { }); describe("ENTITYOPERATIONFAILED", () => { - it("should fail entity call task with error details", async () => { + it("should fail entity call task with EntityOperationFailedException", async () => { // Arrange let caughtError: Error | undefined; const orchestrator = async function* (ctx: OrchestrationContext): AsyncGenerator, string, number> { @@ -225,10 +227,20 @@ describe("OrchestrationExecutor Entity Operation Events", () => { await executor.execute("test-instance", oldEvents2, newEvents2); - // Assert + // Assert - error should be EntityOperationFailedException expect(caughtError).toBeDefined(); + expect(caughtError).toBeInstanceOf(EntityOperationFailedException); + expect(caughtError).toBeInstanceOf(TaskFailedError); expect(caughtError!.message).toContain("badOperation"); expect(caughtError!.message).toContain("Operation not supported"); + + // Verify entity-specific context is preserved + const entityError = caughtError as EntityOperationFailedException; + expect(entityError.entityId.name).toBe("counter"); + expect(entityError.entityId.key).toBe("my-counter"); + expect(entityError.operationName).toBe("badOperation"); + expect(entityError.failureDetails.errorType).toBe("InvalidOperationError"); + expect(entityError.failureDetails.errorMessage).toBe("Operation not supported"); }); it("should propagate failure to orchestration if not caught", async () => { @@ -268,6 +280,49 @@ describe("OrchestrationExecutor Entity Operation Events", () => { const completeAction = completeActionWrapper!.getCompleteorchestration()!; expect(completeAction.getOrchestrationstatus()).toBe(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED); }); + + it("should throw EntityOperationFailedException with TaskFailedError details when uncaught", async () => { + // Arrange — verify the uncaught path produces an EntityOperationFailedException in the + // orchestration failure details message, matching the documented API contract + const orchestrator = async function* (ctx: OrchestrationContext): AsyncGenerator, string, number> { + const entityId = new EntityInstanceId("counter", "my-counter"); + yield ctx.entities.callEntity(entityId, "badOperation"); + return "should not reach here"; + }; + + registry.addNamedOrchestrator("TestOrchestrator", orchestrator); + + const executor = new OrchestrationExecutor(registry); + + const oldEvents: pb.HistoryEvent[] = []; + const newEvents: pb.HistoryEvent[] = [ + ph.newOrchestratorStartedEvent(new Date()), + ph.newExecutionStartedEvent("TestOrchestrator", "test-instance", undefined), + ]; + + const result1 = await executor.execute("test-instance", oldEvents, newEvents); + const requestId = result1.actions[0].getSendentitymessage()!.getEntityoperationcalled()!.getRequestid(); + + // Fail the operation + const oldEvents2 = [...newEvents]; + const newEvents2 = [ + ph.newOrchestratorStartedEvent(new Date()), + newEntityOperationFailedEvent(100, requestId, "ValidationError", "Invalid input"), + ]; + + const result2 = await executor.execute("test-instance", oldEvents2, newEvents2); + + // Assert - orchestration should fail with the EntityOperationFailedException message + const completeActionWrapper = result2.actions.find((a) => a.hasCompleteorchestration()); + expect(completeActionWrapper).toBeDefined(); + const completeAction = completeActionWrapper!.getCompleteorchestration()!; + expect(completeAction.getOrchestrationstatus()).toBe(pb.OrchestrationStatus.ORCHESTRATION_STATUS_FAILED); + const failureDetails = completeAction.getFailuredetails(); + expect(failureDetails).toBeDefined(); + expect(failureDetails!.getErrortype()).toBe("EntityOperationFailedException"); + expect(failureDetails!.getErrormessage()).toContain("badOperation"); + expect(failureDetails!.getErrormessage()).toContain("Invalid input"); + }); }); describe("Multiple entity calls", () => { diff --git a/packages/durabletask-js/test/entity-operation-failed-exception.spec.ts b/packages/durabletask-js/test/entity-operation-failed-exception.spec.ts index 9721df9..9ea9b2f 100644 --- a/packages/durabletask-js/test/entity-operation-failed-exception.spec.ts +++ b/packages/durabletask-js/test/entity-operation-failed-exception.spec.ts @@ -7,6 +7,7 @@ import { TaskFailureDetails, createTaskFailureDetails, } from "../src/entities/entity-operation-failed-exception"; +import { TaskFailedError } from "../src/task/exception/task-failed-error"; import * as pb from "../src/proto/orchestrator_service_pb"; import { StringValue } from "google-protobuf/google/protobuf/wrappers_pb"; @@ -65,6 +66,24 @@ describe("EntityOperationFailedException", () => { expect(exception instanceof EntityOperationFailedException).toBe(true); }); + it("should be instanceof TaskFailedError", () => { + // Arrange + const entityId = new EntityInstanceId("counter", "my-counter"); + const failureDetails: TaskFailureDetails = { + errorType: "Error", + errorMessage: "Something went wrong", + }; + + // Act + const exception = new EntityOperationFailedException(entityId, "op", failureDetails); + + // Assert + expect(exception instanceof TaskFailedError).toBe(true); + expect(exception.details).toBeDefined(); + expect(exception.details.errorType).toBe("Error"); + expect(exception.details.message).toBe("Something went wrong"); + }); + it("should include stack trace", () => { // Arrange const entityId = new EntityInstanceId("counter", "my-counter");