From 4394c6832e4649037a539b722363a8645ed43793 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Fri, 17 Apr 2026 08:58:48 +0000 Subject: [PATCH] Fix V2 entity operationInfos falsy-zero bug When a framework-level exception occurs during V2 entity batch execution, resultsCount is 0 (zero individual results). The expression `resultsCount || operationInfos.length` evaluates to operationInfos.length because 0 is falsy in JavaScript, causing all operationInfos to be included despite there being no corresponding results. Replace with `resultsCount` directly, which is always a number from .length and needs no fallback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/worker/task-hub-grpc-worker.ts | 7 +- .../durabletask-js/test/worker-entity.spec.ts | 133 ++++++++++++++++++ 2 files changed, 138 insertions(+), 2 deletions(-) diff --git a/packages/durabletask-js/src/worker/task-hub-grpc-worker.ts b/packages/durabletask-js/src/worker/task-hub-grpc-worker.ts index d762d52..c7ab5c3 100644 --- a/packages/durabletask-js/src/worker/task-hub-grpc-worker.ts +++ b/packages/durabletask-js/src/worker/task-hub-grpc-worker.ts @@ -967,9 +967,12 @@ export class TaskHubGrpcWorker { // Add V2 operationInfos if provided (used by DTS backend) if (operationInfos && operationInfos.length > 0) { - // Take only as many operationInfos as there are results + // Take only as many operationInfos as there are results. + // Use resultsCount directly (not `resultsCount || operationInfos.length`) + // because 0 is a valid count when a framework-level error produces zero + // individual results; the falsy-OR would incorrectly include all infos. const resultsCount = batchResult.getResultsList().length; - const infosToInclude = operationInfos.slice(0, resultsCount || operationInfos.length); + const infosToInclude = operationInfos.slice(0, resultsCount); batchResult.setOperationinfosList(infosToInclude); } diff --git a/packages/durabletask-js/test/worker-entity.spec.ts b/packages/durabletask-js/test/worker-entity.spec.ts index 7fc73ad..0135921 100644 --- a/packages/durabletask-js/test/worker-entity.spec.ts +++ b/packages/durabletask-js/test/worker-entity.spec.ts @@ -330,4 +330,137 @@ describe("TaskHubGrpcWorker", () => { expect(pendingWorkItems.size).toBe(0); }); }); + + describe("V2 Entity operationInfos handling", () => { + it("should include operationInfos matching result count on successful V2 execution", async () => { + // Arrange + const worker = new TaskHubGrpcWorker({ logger: new NoOpLogger() }); + const factory: EntityFactory = () => new CounterEntity(); + worker.addNamedEntity("counter", factory); + + const mockStub = createMockStub(); + const req = createEntityRequestV2("counter", "key1"); + + // Act + (worker as any)._executeEntityV2(req, COMPLETION_TOKEN, mockStub.stub); + const pendingWorkItems: Set> = (worker as any)._pendingWorkItems; + await Promise.all(pendingWorkItems); + + // Assert - one result per operation, so one operationInfo should be included + const result = mockStub.capturedResult!; + expect(result.getResultsList().length).toBe(1); + expect(result.getOperationinfosList().length).toBe(1); + expect(result.getOperationinfosList()[0].getRequestid()).toBe("req-1"); + }); + + it("should include zero operationInfos when framework error produces zero results", async () => { + // Arrange - register an entity whose factory throws a framework-level error + const worker = new TaskHubGrpcWorker({ logger: new NoOpLogger() }); + const throwingFactory: EntityFactory = () => { + throw new Error("factory explosion"); + }; + worker.addNamedEntity("broken", throwingFactory); + + const mockStub = createMockStub(); + const req = createEntityRequestV2("broken", "key1"); + + // Act + (worker as any)._executeEntityV2(req, COMPLETION_TOKEN, mockStub.stub); + const pendingWorkItems: Set> = (worker as any)._pendingWorkItems; + await Promise.all(pendingWorkItems); + + // Assert - framework error: zero results AND zero operationInfos + const result = mockStub.capturedResult!; + expect(result.getResultsList().length).toBe(0); + expect(result.getOperationinfosList().length).toBe(0); + expect(result.getFailuredetails()).toBeDefined(); + expect(result.getFailuredetails()!.getErrormessage()).toBe("factory explosion"); + }); + + it("should include operationInfos for all results when entity is not found via V2", async () => { + // Arrange - no entity registered for the name in the request + const worker = new TaskHubGrpcWorker({ logger: new NoOpLogger() }); + + const mockStub = createMockStub(); + const req = createEntityRequestV2("nonexistent", "key1"); + + // Act + (worker as any)._executeEntityV2(req, COMPLETION_TOKEN, mockStub.stub); + const pendingWorkItems: Set> = (worker as any)._pendingWorkItems; + await Promise.all(pendingWorkItems); + + // Assert - not-found path creates one error result per operation, so operationInfos should match + const result = mockStub.capturedResult!; + expect(result.getResultsList().length).toBe(1); + expect(result.getOperationinfosList().length).toBe(1); + }); + + it("should include operationInfos for multiple operations on successful V2 execution", async () => { + // Arrange + const worker = new TaskHubGrpcWorker({ logger: new NoOpLogger() }); + const factory: EntityFactory = () => new CounterEntity(); + worker.addNamedEntity("counter", factory); + + const mockStub = createMockStub(); + + // Create a V2 request with multiple operations + const req = new pb.EntityRequest(); + req.setInstanceid("@counter@key1"); + + const event1 = new pb.HistoryEvent(); + const signaled1 = new pb.EntityOperationSignaledEvent(); + signaled1.setOperation("increment"); + signaled1.setRequestid("req-1"); + event1.setEntityoperationsignaled(signaled1); + + const event2 = new pb.HistoryEvent(); + const signaled2 = new pb.EntityOperationSignaledEvent(); + signaled2.setOperation("increment"); + signaled2.setRequestid("req-2"); + event2.setEntityoperationsignaled(signaled2); + + req.setOperationrequestsList([event1, event2]); + + // Act + (worker as any)._executeEntityV2(req, COMPLETION_TOKEN, mockStub.stub); + const pendingWorkItems: Set> = (worker as any)._pendingWorkItems; + await Promise.all(pendingWorkItems); + + // Assert - two results, two operationInfos + const result = mockStub.capturedResult!; + expect(result.getResultsList().length).toBe(2); + expect(result.getOperationinfosList().length).toBe(2); + expect(result.getOperationinfosList()[0].getRequestid()).toBe("req-1"); + expect(result.getOperationinfosList()[1].getRequestid()).toBe("req-2"); + }); + + it("should include zero operationInfos when entity run() throws on V2 execution", async () => { + // Arrange - entity whose run() throws + const worker = new TaskHubGrpcWorker({ logger: new NoOpLogger() }); + + class ThrowingEntity implements ITaskEntity { + run(): never { + throw new Error("run explosion"); + } + } + const factory: EntityFactory = () => new ThrowingEntity(); + worker.addNamedEntity("thrower", factory); + + const mockStub = createMockStub(); + const req = createEntityRequestV2("thrower", "key1"); + + // Act + (worker as any)._executeEntityV2(req, COMPLETION_TOKEN, mockStub.stub); + const pendingWorkItems: Set> = (worker as any)._pendingWorkItems; + await Promise.all(pendingWorkItems); + + // Assert - the entity shim catches per-operation errors and creates per-operation + // failure results, so this should NOT be a framework-level error. + // The entity executor handles operation-level errors internally, + // so we expect one result (failure) and one operationInfo. + const result = mockStub.capturedResult!; + expect(result.getResultsList().length).toBe(1); + expect(result.getOperationinfosList().length).toBe(1); + }); + }); });