Skip to content

Commit 078c05d

Browse files
committed
[copilot-finds] Bug: Entity StateShim.setState() corrupts cache on serialization failure
Fix cache corruption in entity-executor.ts StateShim.setState() where cachedValue was set before JSON.stringify(), causing getState() to return the invalid value instead of the last valid state when serialization fails and entity code catches the error. Changes: - Move cachedValue assignment after serialization succeeds - Invalidate cache in catch block so getState() re-parses last valid state - Preserve original error cause with { cause: e } - Add 3 unit tests for cache preservation, recovery, and error cause Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 05ab1da commit 078c05d

2 files changed

Lines changed: 122 additions & 1 deletion

File tree

packages/durabletask-js/src/worker/entity-executor.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,15 +87,19 @@ class StateShim implements TaskEntityState {
8787
}
8888

8989
setState(state: unknown): void {
90-
this.cachedValue = state;
9190
try {
9291
this.serializedValue =
9392
state != null ? JSON.stringify(state) : undefined;
9493

94+
this.cachedValue = state;
9595
this.cacheValid = true;
9696
} catch (e) {
97+
// Invalidate cache so getState() re-parses from the last valid serializedValue
98+
this.cacheValid = false;
99+
this.cachedValue = undefined;
97100
throw new Error(
98101
`Entity state is not JSON-serializable: ${e instanceof Error ? e.message : String(e)}`,
102+
{ cause: e },
99103
);
100104
}
101105
}

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

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
import { TaskEntityShim } from "../src/worker/entity-executor";
55
import { TaskEntity } from "../src/entities/task-entity";
6+
import { ITaskEntity } from "../src/entities/task-entity";
7+
import { TaskEntityOperation } from "../src/entities/task-entity-operation";
68
import { EntityInstanceId } from "../src/entities/entity-instance-id";
79
import * as pb from "../src/proto/orchestrator_service_pb";
810
import { StringValue } from "google-protobuf/google/protobuf/wrappers_pb";
@@ -433,6 +435,121 @@ describe("TaskEntityShim", () => {
433435
"Entity state is not JSON-serializable",
434436
);
435437
});
438+
439+
it("should preserve previous valid state after failed setState", async () => {
440+
// Entity that reads state via operation.state directly, attempts to set
441+
// non-serializable state, catches the error, then reads state again.
442+
// This tests that the StateShim cache is not corrupted by a failed setState.
443+
class RecoveringEntity implements ITaskEntity {
444+
run(operation: TaskEntityOperation): unknown {
445+
// First read populates the cache (cacheValid = true)
446+
const initialState = operation.state.getState<{ count: number }>();
447+
const initialCount = initialState?.count;
448+
449+
try {
450+
const circular: Record<string, unknown> = {};
451+
circular.self = circular;
452+
operation.state.setState(circular); // Should throw
453+
} catch {
454+
// After failed setState, getState should return the previous valid state
455+
}
456+
457+
const recoveredState = operation.state.getState<{ count: number }>();
458+
return { initialCount, recoveredCount: recoveredState?.count };
459+
}
460+
}
461+
462+
const entity = new RecoveringEntity();
463+
const shim = new TaskEntityShim(entity, entityId);
464+
const request = createBatchRequest(
465+
entityId.toString(),
466+
[{ name: "recover" }],
467+
{ count: 42 },
468+
);
469+
470+
const result = await shim.executeAsync(request);
471+
472+
const opResult = result.getResultsList()[0];
473+
expect(opResult.hasSuccess()).toBe(true);
474+
475+
const output = JSON.parse(opResult.getSuccess()!.getResult()!.getValue());
476+
expect(output.initialCount).toBe(42);
477+
// After failed setState, getState should return 42 (not the circular object)
478+
expect(output.recoveredCount).toBe(42);
479+
});
480+
481+
it("should allow valid setState after a failed setState", async () => {
482+
// Entity that fails a setState call, then successfully sets valid state
483+
class SetAfterFailEntity implements ITaskEntity {
484+
run(operation: TaskEntityOperation): unknown {
485+
// Read state to populate cache
486+
operation.state.getState();
487+
488+
try {
489+
const circular: Record<string, unknown> = {};
490+
circular.self = circular;
491+
operation.state.setState(circular);
492+
} catch {
493+
// Expected - now set valid state
494+
operation.state.setState({ count: 99 });
495+
}
496+
497+
return operation.state.getState<{ count: number }>()?.count;
498+
}
499+
}
500+
501+
const entity = new SetAfterFailEntity();
502+
const shim = new TaskEntityShim(entity, entityId);
503+
const request = createBatchRequest(
504+
entityId.toString(),
505+
[{ name: "setAfterFail" }],
506+
{ count: 10 },
507+
);
508+
509+
const result = await shim.executeAsync(request);
510+
511+
const opResult = result.getResultsList()[0];
512+
expect(opResult.hasSuccess()).toBe(true);
513+
expect(JSON.parse(opResult.getSuccess()!.getResult()!.getValue())).toBe(99);
514+
515+
// Final state should be the recovered value
516+
const finalState = result.getEntitystate()?.getValue();
517+
expect(finalState).toBeDefined();
518+
expect(JSON.parse(finalState!)).toEqual({ count: 99 });
519+
});
520+
521+
it("should preserve error cause in setState error", async () => {
522+
// Verify the error wrapping preserves the original cause
523+
class CauseCheckEntity implements ITaskEntity {
524+
caughtCause: unknown = undefined;
525+
526+
run(operation: TaskEntityOperation): unknown {
527+
try {
528+
const circular: Record<string, unknown> = {};
529+
circular.self = circular;
530+
operation.state.setState(circular);
531+
} catch (e) {
532+
this.caughtCause = e instanceof Error ? e.cause : undefined;
533+
throw e; // Re-throw to record failure
534+
}
535+
return undefined;
536+
}
537+
}
538+
539+
const entity = new CauseCheckEntity();
540+
const shim = new TaskEntityShim(entity, entityId);
541+
const request = createBatchRequest(
542+
entityId.toString(),
543+
[{ name: "checkCause" }],
544+
{},
545+
);
546+
547+
await shim.executeAsync(request);
548+
549+
// The wrapped error should preserve the original TypeError cause
550+
expect(entity.caughtCause).toBeDefined();
551+
expect(entity.caughtCause).toBeInstanceOf(TypeError);
552+
});
436553
});
437554

438555
describe("invalid operation input", () => {

0 commit comments

Comments
 (0)