Skip to content

fix: bound worker hello and cancel reconnect backoff - #353

Open
wangbill (YunchuWang) wants to merge 13 commits into
mainfrom
yunchuwang-fix-worker-startup-timeout
Open

fix: bound worker hello and cancel reconnect backoff#353
wangbill (YunchuWang) wants to merge 13 commits into
mainfrom
yunchuwang-fix-worker-startup-timeout

Conversation

@YunchuWang

@YunchuWang wangbill (YunchuWang) commented Aug 5, 2026

Copy link
Copy Markdown
Member

Summary

  • apply a 30-second grpc-js deadline to every worker sidecar Hello attempt
  • log and retry initial and reconnect Hello failures using the existing event-driven retry path
  • cancel pending Hello calls and reconnect backoff when stop() is called

Review guide

  1. backoff.util.ts: add optional AbortSignal support to the existing delay.
  2. task-hub-grpc-worker.ts: create one signal per start, apply the Hello deadline, pass the signal through existing retries, and abort/cancel it from stop.
  3. Startup/backoff tests: prove deadline, retry, and cancellation behavior.

The final worker diff is 101 additions / 24 deletions. It preserves the existing event-driven worker; there is no WorkerLifecycle, connection-loop rewrite, or drained-stop contract.

Scope

This intentionally preserves the existing non-blocking start() contract and event-driven worker structure. It does not add startup readiness semantics or a drained connection-loop lifecycle, so it is related to but does not close #69.

Validation

  • focused startup/backoff/stream-recovery tests
  • full durabletask-js test suite
  • core build, ESLint, Prettier, and diff checks

Copilot AI lite review requested due to automatic review settings August 5, 2026 17:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR changes TaskHubGrpcWorker.start() semantics so that awaiting startup only resolves after the sidecar “hello” handshake succeeds and the initial work-item stream is confirmed ready, with a configurable startup timeout budget (default 30s). It also wires the new timeout knob through the Azure-managed worker builder and updates docs/changelogs accordingly.

Changes:

  • Make await worker.start() wait for hello + initial work-item stream readiness, with a bounded startupTimeoutMs covering metadata generation, hello, and stream readiness.
  • Add comprehensive unit tests for startup timing, cancellation, late callbacks, and stream recovery behavior.
  • Expose .startupTimeout(milliseconds) on DurableTaskAzureManagedWorkerBuilder and document the new startup semantics.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
README.md Documents new start() readiness semantics and builder timeout configuration.
CHANGELOG.md Notes the core worker startup behavior change.
packages/durabletask-js/src/worker/task-hub-grpc-worker.ts Implements startup readiness gating, startup timeout budget, and generation-based cancellation/cleanup.
packages/durabletask-js/test/worker-startup.spec.ts Adds focused tests covering startup readiness, timeouts, stop-during-start, and late callbacks.
packages/durabletask-js/test/worker-stream-recovery.spec.ts Adjusts mocks to account for metadata-based “stream ready” detection.
packages/durabletask-js-azuremanaged/src/worker-builder.ts Adds .startupTimeout() builder method and passes it into core worker options.
packages/durabletask-js-azuremanaged/test/unit/worker-builder.spec.ts Verifies builder propagates startup timeout to the core worker instance.
packages/durabletask-js-azuremanaged/README.md Documents startup readiness and builder timeout configuration.
packages/durabletask-js-azuremanaged/CHANGELOG.md Notes the new builder API for startup timeout configuration.

Comment thread packages/durabletask-js/src/worker/task-hub-grpc-worker.ts Outdated
Comment thread packages/durabletask-js/src/worker/task-hub-grpc-worker.ts Outdated
Copilot AI review requested due to automatic review settings August 5, 2026 18:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (2)

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

  • The start() @throws doc is too narrow: start() can now throw TimeoutError for metadata generation and work-item stream establishment timeouts as well, not only the hello handshake.
   * @throws {@link TimeoutError} if the hello handshake exceeds `startupTimeoutMs`.

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

  • The TaskHubGrpcWorkerOptions.startupTimeoutMs doc comment no longer matches behavior: the timeout budget is also applied to work-item stream metadata generation and establishing the initial stream, not just hello/metadata. Updating the comment helps avoid confusing API consumers.
  /**
   * Optional timeout in milliseconds for initial metadata generation and the sidecar hello handshake.
   * Defaults to 30000.
   */
  startupTimeoutMs?: number;

Copilot AI review requested due to automatic review settings August 5, 2026 18:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

wangbill (YunchuWang) and others added 4 commits August 10, 2026 15:58
Make worker startup await the initial sidecar stream, bound connection setup with a configurable timeout, and cleanly abort failed or stopped attempts.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Treat an idle stream as ready once hello succeeds and handlers are attached, preserve retryability after client construction failures, and align the recovery E2E with the startup contract.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 65b88a3f-d79a-4a63-8636-cc41c219db63
Add an isolated job that leaves the emulator stopped initially so the worker startup failure and subsequent stream recovery path execute in CI.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 65b88a3f-d79a-4a63-8636-cc41c219db63
Document the startup rejection as a breaking change, align timeout API wording, and preserve the orchestration reuse documentation while rebasing onto main.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 65b88a3f-d79a-4a63-8636-cc41c219db63
@YunchuWang
wangbill (YunchuWang) force-pushed the yunchuwang-fix-worker-startup-timeout branch from bbef765 to 3482ecb Compare August 10, 2026 23:02
Comment thread packages/durabletask-js/src/worker/task-hub-grpc-worker.ts Fixed
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d491ec0d-9ce9-421b-9953-7d179d53625b

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@YunchuWang
wangbill (YunchuWang) force-pushed the yunchuwang-fix-worker-startup-timeout branch from 45003c4 to 82e60fd Compare August 11, 2026 00:17
wangbill (YunchuWang) and others added 2 commits August 10, 2026 17:46
Track direct internal worker runs under the same lifecycle invariants as start and reconnect so stop can drain them and later starts cannot race stale streams.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d491ec0d-9ce9-421b-9953-7d179d53625b
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d491ec0d-9ce9-421b-9953-7d179d53625b
Comment thread .github/workflows/dts-e2e-tests.yaml Outdated
run: npx jest ${{ matrix.test-group.pattern }} --runInBand --detectOpenHandles
timeout-minutes: 15

worker-stream-recovery:

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this tests is separate

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d491ec0d-9ce9-421b-9953-7d179d53625b
@YunchuWang wangbill (YunchuWang) changed the title fix: await worker startup readiness fix: bound and drain worker connection retries Aug 13, 2026
wangbill (YunchuWang) and others added 5 commits August 13, 2026 14:16
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d491ec0d-9ce9-421b-9953-7d179d53625b
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d491ec0d-9ce9-421b-9953-7d179d53625b
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d491ec0d-9ce9-421b-9953-7d179d53625b
Restores the existing event-driven worker structure while keeping per-Hello deadline and stop cancellation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d491ec0d-9ce9-421b-9953-7d179d53625b
Restores the existing event-driven worker structure while keeping per-Hello deadline and stop cancellation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d491ec0d-9ce9-421b-9953-7d179d53625b
@YunchuWang wangbill (YunchuWang) changed the title fix: bound and drain worker connection retries fix: bound worker hello and cancel reconnect backoff Aug 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Worker can silently hang forever when starting

2 participants