Skip to content

Commit 9ee163f

Browse files
authored
fix: preserve V2 entity operation info for index zero (#279)
1 parent 5667569 commit 9ee163f

2 files changed

Lines changed: 137 additions & 2 deletions

File tree

packages/durabletask-js/src/worker/task-hub-grpc-worker.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -977,9 +977,12 @@ export class TaskHubGrpcWorker {
977977

978978
// Add V2 operationInfos if provided (used by DTS backend)
979979
if (operationInfos && operationInfos.length > 0) {
980-
// Take only as many operationInfos as there are results
980+
// Take only as many operationInfos as there are results.
981+
// Use resultsCount directly (not `resultsCount || operationInfos.length`)
982+
// because 0 is a valid count when a framework-level error produces zero
983+
// individual results; the falsy-OR would incorrectly include all infos.
981984
const resultsCount = batchResult.getResultsList().length;
982-
const infosToInclude = operationInfos.slice(0, resultsCount || operationInfos.length);
985+
const infosToInclude = operationInfos.slice(0, resultsCount);
983986
batchResult.setOperationinfosList(infosToInclude);
984987
}
985988

packages/durabletask-js/test/worker-entity.spec.ts

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,4 +330,136 @@ describe("TaskHubGrpcWorker", () => {
330330
expect(pendingWorkItems.size).toBe(0);
331331
});
332332
});
333+
334+
describe("V2 Entity operationInfos handling", () => {
335+
it("should include operationInfos matching result count on successful V2 execution", async () => {
336+
// Arrange
337+
const worker = new TaskHubGrpcWorker({ logger: new NoOpLogger() });
338+
const factory: EntityFactory = () => new CounterEntity();
339+
worker.addNamedEntity("counter", factory);
340+
341+
const mockStub = createMockStub();
342+
const req = createEntityRequestV2("counter", "key1");
343+
344+
// Act
345+
(worker as any)._executeEntityV2(req, COMPLETION_TOKEN, mockStub.stub);
346+
const pendingWorkItems: Set<Promise<void>> = (worker as any)._pendingWorkItems;
347+
await Promise.all(pendingWorkItems);
348+
349+
// Assert - one result per operation, so one operationInfo should be included
350+
const result = mockStub.capturedResult!;
351+
expect(result.getResultsList().length).toBe(1);
352+
expect(result.getOperationinfosList().length).toBe(1);
353+
expect(result.getOperationinfosList()[0].getRequestid()).toBe("req-1");
354+
});
355+
356+
it("should include zero operationInfos when framework error produces zero results", async () => {
357+
// Arrange - register an entity whose factory throws a framework-level error
358+
const worker = new TaskHubGrpcWorker({ logger: new NoOpLogger() });
359+
const throwingFactory: EntityFactory = () => {
360+
throw new Error("factory explosion");
361+
};
362+
worker.addNamedEntity("broken", throwingFactory);
363+
364+
const mockStub = createMockStub();
365+
const req = createEntityRequestV2("broken", "key1");
366+
367+
// Act
368+
(worker as any)._executeEntityV2(req, COMPLETION_TOKEN, mockStub.stub);
369+
const pendingWorkItems: Set<Promise<void>> = (worker as any)._pendingWorkItems;
370+
await Promise.all(pendingWorkItems);
371+
372+
// Assert - framework error: zero results AND zero operationInfos
373+
const result = mockStub.capturedResult!;
374+
expect(result.getResultsList().length).toBe(0);
375+
expect(result.getOperationinfosList().length).toBe(0);
376+
expect(result.getFailuredetails()).toBeDefined();
377+
expect(result.getFailuredetails()!.getErrormessage()).toBe("factory explosion");
378+
});
379+
380+
it("should include operationInfos for all results when entity is not found via V2", async () => {
381+
// Arrange - no entity registered for the name in the request
382+
const worker = new TaskHubGrpcWorker({ logger: new NoOpLogger() });
383+
384+
const mockStub = createMockStub();
385+
const req = createEntityRequestV2("nonexistent", "key1");
386+
387+
// Act
388+
(worker as any)._executeEntityV2(req, COMPLETION_TOKEN, mockStub.stub);
389+
const pendingWorkItems: Set<Promise<void>> = (worker as any)._pendingWorkItems;
390+
await Promise.all(pendingWorkItems);
391+
392+
// Assert - not-found path creates one error result per operation, so operationInfos should match
393+
const result = mockStub.capturedResult!;
394+
expect(result.getResultsList().length).toBe(1);
395+
expect(result.getOperationinfosList().length).toBe(1);
396+
});
397+
398+
it("should include operationInfos for multiple operations on successful V2 execution", async () => {
399+
// Arrange
400+
const worker = new TaskHubGrpcWorker({ logger: new NoOpLogger() });
401+
const factory: EntityFactory = () => new CounterEntity();
402+
worker.addNamedEntity("counter", factory);
403+
404+
const mockStub = createMockStub();
405+
406+
// Create a V2 request with multiple operations
407+
const req = new pb.EntityRequest();
408+
req.setInstanceid("@counter@key1");
409+
410+
const event1 = new pb.HistoryEvent();
411+
const signaled1 = new pb.EntityOperationSignaledEvent();
412+
signaled1.setOperation("increment");
413+
signaled1.setRequestid("req-1");
414+
event1.setEntityoperationsignaled(signaled1);
415+
416+
const event2 = new pb.HistoryEvent();
417+
const signaled2 = new pb.EntityOperationSignaledEvent();
418+
signaled2.setOperation("increment");
419+
signaled2.setRequestid("req-2");
420+
event2.setEntityoperationsignaled(signaled2);
421+
422+
req.setOperationrequestsList([event1, event2]);
423+
424+
// Act
425+
(worker as any)._executeEntityV2(req, COMPLETION_TOKEN, mockStub.stub);
426+
const pendingWorkItems: Set<Promise<void>> = (worker as any)._pendingWorkItems;
427+
await Promise.all(pendingWorkItems);
428+
429+
// Assert - two results, two operationInfos
430+
const result = mockStub.capturedResult!;
431+
expect(result.getResultsList().length).toBe(2);
432+
expect(result.getOperationinfosList().length).toBe(2);
433+
expect(result.getOperationinfosList()[0].getRequestid()).toBe("req-1");
434+
expect(result.getOperationinfosList()[1].getRequestid()).toBe("req-2");
435+
});
436+
437+
it("should include operationInfos matching result count when entity run() throws on V2 execution", async () => {
438+
const worker = new TaskHubGrpcWorker({ logger: new NoOpLogger() });
439+
440+
class ThrowingEntity implements ITaskEntity {
441+
run(): never {
442+
throw new Error("run explosion");
443+
}
444+
}
445+
const factory: EntityFactory = () => new ThrowingEntity();
446+
worker.addNamedEntity("thrower", factory);
447+
448+
const mockStub = createMockStub();
449+
const req = createEntityRequestV2("thrower", "key1");
450+
451+
// Act
452+
(worker as any)._executeEntityV2(req, COMPLETION_TOKEN, mockStub.stub);
453+
const pendingWorkItems: Set<Promise<void>> = (worker as any)._pendingWorkItems;
454+
await Promise.all(pendingWorkItems);
455+
456+
// Assert - the entity shim catches per-operation errors and creates per-operation
457+
// failure results, so this should NOT be a framework-level error.
458+
// The entity executor handles operation-level errors internally,
459+
// so we expect one result (failure) and one operationInfo.
460+
const result = mockStub.capturedResult!;
461+
expect(result.getResultsList().length).toBe(1);
462+
expect(result.getOperationinfosList().length).toBe(1);
463+
});
464+
});
333465
});

0 commit comments

Comments
 (0)