Skip to content

[DurableTask.ServiceBus] Honor operationTimeout in SessionClient.AcceptMessageSessionAsync - #1426

Open
Davide Montanari (davidemontanari) wants to merge 1 commit into
Azure:mainfrom
davidemontanari:davidemontanari/dtfx-sb-honor-accept-session-timeout
Open

Davide Montanari (davidemontanari) wants to merge 1 commit into
Azure:mainfrom
davidemontanari:davidemontanari/dtfx-sb-honor-accept-session-timeout

Conversation

@davidemontanari

Copy link
Copy Markdown
Member

Summary

SessionClient.AcceptMessageSessionAsync(TimeSpan operationTimeout) ignored its operationTimeout parameter. The call to ServiceBusClient.AcceptNextSessionAsync was not given a cancellation token, so an empty accept waited for the SDK's TryTimeout (60s by default; this project doesn't configure it) instead of the timeout the caller requested.

Callers (LockNextTaskOrchestrationWorkItemAsync and FetchTrackingWorkItemAsync) pass the dispatcher's DefaultReceiveTimeout (30s), so the effective wait was twice what the dispatcher reports in its StartFetch / FetchWorkItemStarting traces.

Changes

  • AcceptMessageSessionAsync now creates a CancellationTokenSource(operationTimeout) when operationTimeout > TimeSpan.Zero and passes its token to AcceptNextSessionAsync.
  • If that timeout cancels the accept, the method returns null, the existing "no session available" result, the same as the existing ServiceTimeout path. The dispatcher re-polls immediately with no backoff, as before.
  • The catch is filtered with when (timeoutCts.IsCancellationRequested), so cancellations from other sources are still propagated.
  • A non-positive operationTimeout keeps the previous behavior, where the wait is limited only by the SDK's TryTimeout.

Impact

  • Pickup latency for new work is unchanged: AcceptNextSessionAsync is a long poll and returns as soon as a session becomes available.
  • When idle, each fetcher polls about every 30s instead of about every 60s, so there are slightly more link attach/detach operations.
  • If the timeout fires just as a session is accepted, the SDK closes the link and releases the session lock; the session is picked up on the next poll. Accepting a session does not consume messages.

Tests

Added SessionClientTests (net8.0 and net48), using fake ServiceBusClient / ServiceBusSessionReceiver subclasses:

  • returns the session when one is accepted;
  • returns null shortly after operationTimeout elapses, and the accept token is cancelled;
  • returns null on ServiceBusFailureReason.ServiceTimeout;
  • does not cancel when operationTimeout is non-positive;
  • propagates OperationCanceledException not caused by the timeout;
  • propagates other ServiceBusExceptions.

Without the fix, the timeout test fails (the call returns after about 10s); all 6 tests pass with it.

Copilot AI balanced review requested due to automatic review settings October 22, 2026 22:21
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

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.

🟢 Approval recommended

Static review found no blocking issues in the focused timeout change or regression tests; tests were not executed.

No open issues 🎉

What changed in this PR

Updates the Service Bus session abstraction to honor the caller’s receive timeout instead of relying solely on the SDK timeout.

Changes:

  • Passes a timed cancellation token when operationTimeout is positive.
  • Returns null on timeout while preserving non-positive timeout behavior and propagating unrelated errors.
  • Adds six regression tests covering successful acceptance, timeout handling, and error propagation.
File Description
test/​DurableTask.ServiceBus.Tests/​SessionClientTests.cs Adds fake-client tests for session acceptance and timeout behavior.
src/​DurableTask.ServiceBus/​Common/​Abstraction/​ServiceBusAbstraction.cs Enforces the requested session-accept timeout and documents fallback behavior.

Review effort: Balanced

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.

2 participants