Address review comments

Author: pombosilva

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

@@ -74,43 +74,37 @@ class InstanceImpl implements WorkflowInstance {
 	}
 
 	public async pause(): Promise<void> {
-		const instance = await this.getInstance();
+		using instance = await this.getInstance();
 		await instance.pause();
-		instance[Symbol.dispose]();
 	}
 
 	public async resume(): Promise<void> {
-		const instance = await this.getInstance();
+		using instance = await this.getInstance();
 		await instance.resume();
-		instance[Symbol.dispose]();
 	}
 
 	public async terminate(): Promise<void> {
-		const instance = await this.getInstance();
+		using instance = await this.getInstance();
 		await instance.terminate();
-		instance[Symbol.dispose]();
 	}
 
 	public async restart(): Promise<void> {
-		const instance = await this.getInstance();
+		using instance = await this.getInstance();
 		await instance.restart();
-		instance[Symbol.dispose]();
 	}
 
 	public async status(): Promise<InstanceStatus> {
-		const instance = await this.getInstance();
+		using instance = await this.getInstance();
 		using res = (await instance.status()) as InstanceStatus & Disposable;
-		instance[Symbol.dispose]();
 		return structuredClone(res);
 	}
 
 	public async sendEvent(args: {
 		payload: unknown;
 		type: string;
 	}): Promise<void> {
-		const instance = await this.getInstance();
+		using instance = await this.getInstance();
 		await instance.sendEvent(args);
-		instance[Symbol.dispose]();
 	}
 }
 

packages/workflows-shared/src/binding.ts

@@ -219,20 +219,31 @@ export class WorkflowHandle extends RpcTarget implements WorkflowInstance {
 	public async status(): Promise<
 		InstanceStatus & { __LOCAL_DEV_STEP_OUTPUTS: unknown[] }
 	> {
-		// If the stub is stale (e.g. after pause/restart/terminate aborted
-		// the DO), refresh it transparently so callers never see the error.
-		let status: EngineInstanceStatus;
+		// Both getStatus() and readLogs() must use the same fresh stub.
+		// After pause/restart/terminate aborts the DO, the stub goes stale
+		const fetchStatusAndLogs = async () => {
+			const status = await this.stub.getStatus();
+
+			// NOTE(lduarte): for some reason, sync functions over RPC are typed as never instead of Promise<EngineLogs>
+			const logs = await (this.stub.readLogs() as unknown as Promise<
+				EngineLogs & Disposable
+			>);
+
+			return { status, logs };
+		};
+
+		let result: {
+			status: EngineInstanceStatus;
+			logs: EngineLogs & Disposable;
+		};
 		try {
-			status = await this.stub.getStatus();
+			result = await fetchStatusAndLogs();
 		} catch {
 			this.stub = this.getStub();
-			status = await this.stub.getStatus();
+			result = await fetchStatusAndLogs();
 		}
-
-		// NOTE(lduarte): for some reason, sync functions over RPC are typed as never instead of Promise<EngineLogs>
-		using logs = await (this.stub.readLogs() as unknown as Promise<
-			EngineLogs & Disposable
-		>);
+		// Dispose the RPC handle when the method scope exits
+		using logs = result.logs;
 
 		const filteredLogs = logs.logs.filter(
 			(log) =>
@@ -255,7 +266,7 @@ export class WorkflowHandle extends RpcTarget implements WorkflowInstance {
 		)?.metadata.error;
 
 		return {
-			status: instanceStatusName(status),
+			status: instanceStatusName(result.status),
 			__LOCAL_DEV_STEP_OUTPUTS: stepOutputs,
 			output: workflowOutput,
 			error: workflowError,

packages/workflows-shared/src/engine.ts

@@ -12,6 +12,7 @@ import { computeHash } from "./lib/cache";
 import {
 	ABORT_REASONS,
 	createWorkflowError,
+	isAbortError,
 	isUserTriggeredPause,
 	WorkflowFatalError,
 } from "./lib/errors";
@@ -646,15 +647,22 @@ export class Engine extends DurableObject<Env> {
 			InstanceStatus.WaitingForPause
 		);
 
-		void this.timeoutHandler.waitUntilNothingIsRunning("pause", async () => {
-			await this.ctx.storage.put(PAUSE_DATETIME, new Date());
-			await this.setStatus(
-				metadata.accountId,
-				metadata.instance.id,
-				InstanceStatus.Paused
-			);
-			await this.abort(ABORT_REASONS.USER_PAUSE);
-		});
+		void this.timeoutHandler
+			.waitUntilNothingIsRunning("pause", async () => {
+				await this.ctx.storage.put(PAUSE_DATETIME, new Date());
+				await this.setStatus(
+					metadata.accountId,
+					metadata.instance.id,
+					InstanceStatus.Paused
+				);
+				await this.abort(ABORT_REASONS.USER_PAUSE);
+			})
+			.catch((e) => {
+				// Expected: abort rejects the promise chain when it kills the DO
+				if (!isAbortError(e)) {
+					throw e;
+				}
+			});
 	}
 
 	async userTriggeredRestart() {
@@ -808,6 +816,7 @@ export class Engine extends DurableObject<Env> {
 				InstanceStatus.Errored, // TODO (WOR-85): Remove this once upgrade story is done
 				InstanceStatus.Terminated,
 				InstanceStatus.Complete,
+				InstanceStatus.Paused,
 			].includes(status)
 		) {
 			return;