Skip to content

Add JsonRpc outbound request timeout#1471

Merged
AArnott merged 4 commits into
mainfrom
aarnott-plan-outbound-rpc-timeout
Jun 23, 2026
Merged

Add JsonRpc outbound request timeout#1471
AArnott merged 4 commits into
mainfrom
aarnott-plan-outbound-rpc-timeout

Conversation

@AArnott

@AArnott AArnott commented Jun 23, 2026

Copy link
Copy Markdown
Member

Why

Issue #1443 asks for a connection-wide way to bound outbound RPC calls instead of requiring each call site to create and dispose a linked CancellationTokenSource manually.

Fixes #1443

What changed

  • Added JsonRpc.OutboundRequestTimeout as a nullable, disabled-by-default setting for response-expected outbound calls.
  • Applied the timeout in the shared outbound invoke path so direct Invoke* APIs and proxy-generated calls behave the same way.
  • Timeout-origin failures now throw TimeoutException and include nameof(JsonRpc.OutboundRequestTimeout) in the message so tests can assert on invariant text instead of localized English.
  • When the caller's token is canceled, OperationCanceledException now escapes with the caller's original token rather than an internal linked token.
  • Timeout-triggered cancellation still goes through the outbound cancellation strategy, so $/cancelRequest is sent even though the caller stops waiting immediately.
  • Notifications remain unchanged.

Notes

The timeout only stops waiting on the client side. The server may still be processing the request, and the PR keeps that behavior explicit in the thrown timeout message.

Testing

  • dotnet build -c Release
  • Targeted timeout/cancellation tests on net9.0, net8.0, and net472
  • Broader cancellation-focused regression sweep on net9.0 with TestCategory=FailsInCloudTest excluded

Add a JsonRpc-wide OutboundRequestTimeout that applies to response-expected outbound calls, turns timeout-origin failures into TimeoutException, preserves caller cancellation tokens, and exercises the behavior across direct, proxy, and cancellation-strategy tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 23, 2026 14:57

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds a connection-wide outbound invocation timeout (JsonRpc.OutboundRequestTimeout) so both direct Invoke* APIs and proxy-generated calls can be bounded without per-call linked CancellationTokenSource management, with updated exception behavior and accompanying tests.

Changes:

  • Introduces JsonRpc.OutboundRequestTimeout (nullable, disabled by default) with validation and documentation.
  • Applies the timeout in the shared outbound invoke path and translates timeout-triggered cancellation to TimeoutException (with invariant message text containing nameof(JsonRpc.OutboundRequestTimeout)).
  • Adds coverage for timeout behavior across direct invokes, proxies, notifications, and cancellation-strategy interactions.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
test/StreamJsonRpc.Tests/JsonRpcTests.cs Adds direct invoke tests for outbound timeout behavior and precedence vs caller cancellation.
test/StreamJsonRpc.Tests/JsonRpcProxyGenerationTests.cs Adds a proxy-call test verifying the JsonRpc-level outbound timeout is honored.
test/StreamJsonRpc.Tests/JsonRpcClient20InteropTests.cs Adds a test ensuring notifications ignore outbound timeout.
test/StreamJsonRpc.Tests/CustomCancellationStrategyTests.cs Adds a test verifying timeout-triggered cancellation still routes through cancellation strategy behavior.
src/StreamJsonRpc/Resources.resx Adds resource strings for positive TimeSpan validation and timeout exception messaging.
src/StreamJsonRpc/JsonRpc.cs Adds OutboundRequestTimeout and integrates timeout cancellation + exception shaping into outbound invoke pipeline.

Comment thread src/StreamJsonRpc/JsonRpc.cs
Comment thread test/StreamJsonRpc.Tests/CustomCancellationStrategyTests.cs Outdated
Comment thread test/StreamJsonRpc.Tests/JsonRpcTests.cs Outdated
Comment thread test/StreamJsonRpc.Tests/JsonRpcProxyGenerationTests.cs Outdated
AArnott and others added 2 commits June 23, 2026 09:15
Translate timeout-driven send-path cancellations into TimeoutException, align most timeout tests with ExpectedTimeout, and keep the custom cancellation-strategy timeout at 100ms because the larger shared timeout hangs on net472 in that scenario.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 23, 2026 16:20

Copilot AI left a comment

Copy link
Copy Markdown

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 7 out of 7 changed files in this pull request and generated 2 comments.

Comment thread src/StreamJsonRpc/JsonRpc.cs Outdated
Comment thread src/StreamJsonRpc/JsonRpc.cs
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@AArnott AArnott enabled auto-merge (squash) June 23, 2026 17:36
@AArnott AArnott merged commit 67708eb into main Jun 23, 2026
9 checks passed
@AArnott AArnott deleted the aarnott-plan-outbound-rpc-timeout branch June 23, 2026 17:42
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.

Allow Configuration of Timeout for Outgoing Method Invocation on Proxies

3 participants