Skip to content

Commit 7e7db5d

Browse files
committed
fix: Use event-driven synchronization instead of fixed delay in stop()
- Replace 10ms sleep with awaiting stream close/end/error events - Add 1s timeout fallback for stream closure - Fix inconsistent JSDoc indentation in client-builder.ts
1 parent 77c2667 commit 7e7db5d

File tree

2 files changed

+23
-4
lines changed

2 files changed

+23
-4
lines changed

packages/durabletask-js-azuremanaged/src/client-builder.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,8 @@ export class DurableTaskAzureManagedClientBuilder {
125125
}
126126

127127
/**
128-
* Sets the logger to use for logging.
129-
* Defaults to ConsoleLogger.
128+
* Sets the logger to use for logging.
129+
* Defaults to ConsoleLogger.
130130
*
131131
* @param logger The logger instance.
132132
* @returns This builder instance.

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

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,8 +282,27 @@ export class TaskHubGrpcWorker {
282282
// Cancel stream first while error handlers are still attached
283283
// This allows the error handler to suppress CANCELLED errors
284284
this._responseStream?.cancel();
285-
// Brief pause to let cancellation error propagate to handlers
286-
await sleep(10);
285+
286+
// Wait for the stream to react to cancellation using events rather than a fixed delay.
287+
// This avoids race conditions caused by relying on timing alone.
288+
if (this._responseStream) {
289+
try {
290+
await withTimeout(
291+
new Promise<void>((resolve) => {
292+
const stream = this._responseStream!;
293+
// Any of these events indicates the stream has processed cancellation / is closing.
294+
stream.once("end", resolve);
295+
stream.once("close", resolve);
296+
stream.once("error", () => resolve());
297+
}),
298+
1000,
299+
"Timed out waiting for response stream to close after cancellation",
300+
);
301+
} catch {
302+
// If we time out waiting for the stream to close, proceed with forced cleanup below.
303+
}
304+
}
305+
287306
// Now safe to remove listeners and destroy
288307
this._responseStream?.removeAllListeners();
289308
this._responseStream?.destroy();

0 commit comments

Comments
 (0)