Skip to content

feat: Built-in request timeout interceptor without Polly dependency (ITimeoutRequest)#154

Merged
samtrion merged 2 commits intomainfrom
copilot/add-timeout-request-interceptor
Mar 28, 2026
Merged

feat: Built-in request timeout interceptor without Polly dependency (ITimeoutRequest)#154
samtrion merged 2 commits intomainfrom
copilot/add-timeout-request-interceptor

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 28, 2026

Adds a BCL-only per-request timeout mechanism via a linked CancellationTokenSource, filling the gap for projects that need deadline enforcement without pulling in NetEvolve.Pulse.Polly.

Changes

NetEvolve.Pulse.Extensibility

  • ITimeoutRequest — marker interface; implement alongside ICommand<T>/IQuery<T> to declare a per-request deadline via TimeSpan? Timeout { get; } (nullable — returning null defers to the global fallback)

NetEvolve.Pulse

  • TimeoutRequestInterceptor<TRequest, TResponse> — only activates for requests implementing ITimeoutRequest; requests that do not implement ITimeoutRequest always pass through regardless of any global timeout setting; creates a linked CTS, throws TimeoutException on deadline exceeded, and correctly distinguishes timeout from caller cancellation (OperationCanceledException)
  • TimeoutRequestInterceptorOptionsTimeSpan? GlobalTimeout applies as a fallback only to ITimeoutRequest implementations that return null from Timeout
  • TimeoutMediatorConfiguratorExtensions.AddRequestTimeout() — fluent registration; per-request ITimeoutRequest.Timeout takes precedence over the global fallback; non-ITimeoutRequest requests are never affected

Tests

  • 9 unit tests covering: within-deadline success, timeout exceeded, caller cancellation disambiguation, nullable-Timeout with global fallback, explicit-Timeout overrides global, non-ITimeoutRequest always passes through, and CTS disposal guarantee
  • 7 integration tests verifying end-to-end timeout behavior in the full mediator pipeline

Usage

// Only ITimeoutRequest-implementing requests with a non-null Timeout are affected
services.AddPulse(c => c.AddRequestTimeout());

// ITimeoutRequest requests that return null from Timeout fall back to 30 s
services.AddPulse(c => c.AddRequestTimeout(TimeSpan.FromSeconds(30)));

// Explicit per-request timeout
public record ProcessOrderCommand(string OrderId) : ICommand<OrderResult>, ITimeoutRequest
{
    public string? CorrelationId { get; set; }
    public TimeSpan? Timeout => TimeSpan.FromSeconds(10);
}

// Defer to the global fallback
public record GetStatusQuery(string Id) : IQuery<Status>, ITimeoutRequest
{
    public string? CorrelationId { get; set; }
    public TimeSpan? Timeout => null;
}

📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.

Copilot AI linked an issue Mar 28, 2026 that may be closed by this pull request
11 tasks
Copilot AI changed the title [WIP] Add built-in request timeout interceptor without Polly dependency feat: Built-in request timeout interceptor without Polly dependency (ITimeoutRequest) Mar 28, 2026
Copilot AI requested a review from samtrion March 28, 2026 01:20
Copilot AI requested a review from samtrion March 28, 2026 07:22
@samtrion samtrion force-pushed the copilot/add-timeout-request-interceptor branch 2 times, most recently from 1bab4f4 to 7439b56 Compare March 28, 2026 08:24
@samtrion samtrion marked this pull request as ready for review March 28, 2026 08:28
@samtrion samtrion requested a review from a team as a code owner March 28, 2026 08:28
Comment on lines +8 to +12
/// When <see cref="GlobalTimeout"/> is set, all requests that do not implement
/// <see cref="Extensibility.ITimeoutRequest"/> are also subject to the global deadline.
/// Requests that implement <see cref="Extensibility.ITimeoutRequest"/> always use their own
/// <see cref="Extensibility.ITimeoutRequest.Timeout"/> value, which takes precedence over
/// <see cref="GlobalTimeout"/>.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The remarks claim GlobalTimeout affects requests not implementing ITimeoutRequest, but interceptor control flow always bypasses those requests, so this documented behavior is impossible.

Show fix
Suggested change
/// When <see cref="GlobalTimeout"/> is set, all requests that do not implement
/// <see cref="Extensibility.ITimeoutRequest"/> are also subject to the global deadline.
/// Requests that implement <see cref="Extensibility.ITimeoutRequest"/> always use their own
/// <see cref="Extensibility.ITimeoutRequest.Timeout"/> value, which takes precedence over
/// <see cref="GlobalTimeout"/>.
/// When <see cref="GlobalTimeout"/> is set, it applies as a fallback to requests that implement
/// <see cref="Extensibility.ITimeoutRequest"/> but return <see langword="null"/> from their
/// <see cref="Extensibility.ITimeoutRequest.Timeout"/> property.
/// Requests that implement <see cref="Extensibility.ITimeoutRequest"/> and return a non-null
/// <see cref="Extensibility.ITimeoutRequest.Timeout"/> value always use their own timeout.
/// Requests that do not implement <see cref="Extensibility.ITimeoutRequest"/> are not subject
/// to any timeout enforcement.
Details

✨ AI Reasoning
​The codebase defines that only opted-in requests participate in timeout enforcement, but this documentation states that requests without opt-in are also affected by the global timeout. That creates an impossible expectation for consumers because the documented path cannot occur under the current control flow.

Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

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.

@copilot resolve this issue

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 28, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.40%. Comparing base (8ef9dbe) to head (cae0f7c).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #154      +/-   ##
==========================================
+ Coverage   85.19%   85.40%   +0.20%     
==========================================
  Files          42       44       +2     
  Lines        1385     1411      +26     
  Branches      125      129       +4     
==========================================
+ Hits         1180     1205      +25     
- Misses        168      169       +1     
  Partials       37       37              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI and others added 2 commits March 28, 2026 17:39
@samtrion samtrion force-pushed the copilot/add-timeout-request-interceptor branch from 7439b56 to cae0f7c Compare March 28, 2026 16:39
@samtrion samtrion merged commit ca6bbcc into main Mar 28, 2026
11 of 12 checks passed
@samtrion samtrion deleted the copilot/add-timeout-request-interceptor branch March 28, 2026 16:51
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.

feat: Built-in request timeout interceptor without Polly dependency (ITimeoutRequest)

2 participants