Skip to content

Commit 14f60e8

Browse files
Workflow binding: remove dispose on void returning method (#11084)
1 parent 9c5601a commit 14f60e8

File tree

3 files changed

+98
-2
lines changed

3 files changed

+98
-2
lines changed

.changeset/seven-camels-guess.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"miniflare": patch
3+
---
4+
5+
remove explicit disposal on void-returning rpc method on workflow binding. This was adding extra dispose calls that were not needed, making the runtime noisy for the customers that saw internal errors unrelated to their code.

packages/miniflare/src/workers/workflows/wrapped-binding.worker.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,8 @@ class InstanceImpl implements WorkflowInstance {
9595
}): Promise<void> {
9696
const instance = (await this.binding.get(this.id)) as WorkflowInstance &
9797
Disposable;
98-
const res = (await instance.sendEvent(args)) as void & Disposable;
98+
await instance.sendEvent(args);
9999
instance[Symbol.dispose]();
100-
res[Symbol.dispose]();
101100
}
102101
}
103102

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
import {
2+
createExecutionContext,
3+
env,
4+
runInDurableObject,
5+
} from "cloudflare:test";
6+
import { describe, expect, it, vi } from "vitest";
7+
import { WorkflowBinding } from "../src/binding";
8+
import type { Engine } from "../src/engine";
9+
import type { ProvidedEnv } from "cloudflare:test";
10+
import type { WorkflowEvent, WorkflowStep } from "cloudflare:workers";
11+
12+
async function setWorkflowEntrypoint(
13+
stub: DurableObjectStub<Engine>,
14+
callback: (event: unknown, step: WorkflowStep) => Promise<unknown>
15+
) {
16+
const ctx = createExecutionContext();
17+
await runInDurableObject(stub, (instance) => {
18+
// @ts-expect-error this is only a stub for WorkflowEntrypoint
19+
instance.env.USER_WORKFLOW = new (class {
20+
constructor(
21+
// eslint-disable-next-line @typescript-eslint/no-shadow
22+
protected ctx: ExecutionContext,
23+
// eslint-disable-next-line @typescript-eslint/no-shadow
24+
protected env: ProvidedEnv
25+
) {}
26+
public async run(
27+
event: Readonly<WorkflowEvent<unknown>>,
28+
step: WorkflowStep
29+
): Promise<unknown> {
30+
return await callback(event, step);
31+
}
32+
})(ctx, env);
33+
});
34+
}
35+
36+
describe("WorkflowBinding", () => {
37+
it("should not call dispose when sending an event to an instance", async () => {
38+
const instanceId = "test-instance-with-event";
39+
const ctx = createExecutionContext();
40+
41+
const binding = new WorkflowBinding(ctx, {
42+
ENGINE: env.ENGINE,
43+
BINDING_NAME: "TEST_WORKFLOW",
44+
});
45+
46+
// Set up a workflow that waits for an event
47+
const engineId = env.ENGINE.idFromName(instanceId);
48+
const engineStub = env.ENGINE.get(engineId);
49+
50+
await setWorkflowEntrypoint(engineStub, async (event, step) => {
51+
const receivedEvent = await step.waitForEvent("wait-for-test-event", {
52+
type: "test-event",
53+
timeout: "10 seconds",
54+
});
55+
return receivedEvent;
56+
});
57+
58+
const { id } = await binding.create({ id: instanceId });
59+
expect(id).toBe(instanceId);
60+
61+
const instance = await binding.get(instanceId);
62+
expect(instance.id).toBe(instanceId);
63+
64+
const disposeSpy = vi.fn();
65+
66+
await runInDurableObject(engineStub, (engine) => {
67+
const originalReceiveEvent = engine.receiveEvent.bind(engine);
68+
engine.receiveEvent = (event) => {
69+
const result = originalReceiveEvent(event);
70+
return Object.assign(result, {
71+
[Symbol.dispose]: disposeSpy,
72+
});
73+
};
74+
});
75+
76+
using _ = (await instance.sendEvent({
77+
type: "test-event",
78+
payload: { test: "data" },
79+
})) as unknown as Disposable;
80+
81+
// Wait a bit to ensure event processing
82+
await vi.waitFor(
83+
async () => {
84+
const status = await instance.status();
85+
expect(status.status).toBe("complete");
86+
},
87+
{ timeout: 1000 }
88+
);
89+
90+
expect(disposeSpy).not.toHaveBeenCalled();
91+
});
92+
});

0 commit comments

Comments
 (0)