feat: add the request/response pipeline and core policies#6
Merged
Conversation
Introduces PipelineStage (sparse enum with pillar/non-pillar semantics), HttpPipelinePolicy (abstract base; async-only in v1), and PipelineRunner (immutable readonly struct that advances the policy chain and terminates at the transport). Tests verify stage-order execution and re-entrancy. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Introduces PipelineBuilder with fluent Add/InsertBefore<T>/InsertAfter<T>/ Replace<T>/Remove<T> operations. Build stable-sorts by stage, validates pillar cardinality (throws InvalidOperationException naming the stage when violated), and produces an HttpPipeline. Also adds PipelineStageHelper (internal IsPillar/PillarStages) and HttpPipeline (entry point with async SendAsync and blocking Send bridge). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds HttpPipelineTests covering SendAsync and the blocking Send bridge. The implementation was already landed with PipelineBuilder in the prior commit; this commit captures the Group 3 test coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace tautological NotNull-only assertions in PipelineBuilderTests with execution-log tests that exercise the actual pipeline chain. New tests confirm: stage-sort order (Operation → Redirect → Diagnostics) via recording stubs, InsertAfter/InsertBefore within the same stage, Replace swapping one policy for another, and Remove dropping the target while leaving others intact. Also fixes the import ordering that caused dotnet format --verify-no-changes to fail (Http.Response was listed before Http.Request; corrected to alphabetical order and added the missing Configuration using directive required by the new tests). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Stamps a fresh RFC 1123 Date header on every request attempt (PerAttempt stage), replacing any prior value. Accepts an optional TimeProvider for deterministic testing; defaults to TimeProvider.System. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Stamps the User-Agent header on every outgoing request (PerCall stage), replacing any prior value. Reads the agent string from DexpaceClientOptions.UserAgent so callers can override it without subclassing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Stamps an Idempotency-Key header (GUID v4) on outgoing requests for configured HTTP methods (default: POST). The key is generated once per logical call, stashed in the PipelineContext property bag under "dexpace.idempotency-key", and reused on retry/redirect re-runs of the same context. Existing caller-supplied keys are preserved. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Throws HttpResponseException on non-2xx responses, draining up to 1 MiB of the error body into a buffered ResponseBody so that HttpResponseException.GetErrorAsync<T> can deserialize it without a StreamConsumedException. Headers, Protocol, and ContentType are carried through to the buffered response on the exception. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add OperationPolicy (PipelineStage.Operation) which applies the overall per-call deadline from DexpaceClientOptions.OverallTimeout. When a positive TimeSpan is configured, a linked CancellationTokenSource is created, armed with CancelAfter, and PipelineContext.CancellationToken is swapped in so every policy and the transport downstream observe the deadline. Non-positive or null values are a no-op (no CTS allocated, no latency cost). Also adds an internal setter to PipelineContext.CancellationToken so the policy can replace the token without reflection. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add RetryPolicy (PipelineStage.Retry) with full-jitter exponential backoff and Retry-After header support. Retries on status codes 408, 429, 500, 502, 503, 504 and on ServiceRequestException / ServiceResponseException. Non- idempotent methods are only retried when the body is replayable and RetryNonIdempotentWhenReplayable is enabled. OperationCanceledException is explicitly excluded from the retry predicate and always propagates. Retry-After is parsed as delta-seconds or RFC 1123 HTTP-date; the HTTP-date path uses the injected TimeProvider for testable "now" lookups. Delays use Task.Delay(TimeSpan, TimeProvider, CancellationToken) so test suites can substitute an InstantTimeProvider that fires timers after 1 ms. The overflow-safe exponent cap (shift <= 30) prevents TimeSpan overflow at high attempt counts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Implements 3xx redirect following per RedirectOptions: - 303 always becomes GET + no body - 301/302 on POST becomes GET + no body (legacy browser behavior) - 307/308 and non-POST 301/302 preserve method and body - HTTPS→HTTP downgrade blocked unless AllowHttpsToHttpDowngrade - Relative Location headers resolved against current request URL - MaxRedirects cap enforced; last 3xx returned to caller when exhausted - Non-replayable bodies block body-preserving redirects - Cross-origin hops strip Authorization and Cookie when StripSensitiveHeadersOnCrossOrigin Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Overflow-safe exponential cap: saturate before shifting so a large BaseDelay never wraps negative. Guard is now baseTicks <= (maxTicks >> shift) ? baseTicks << shift : maxTicks, which is always in [0, MaxDelay] without a multiplication overflow. - Remove dead always-true conjunct (ex is not OperationCanceledException) from IsRetryableException; the type check already excludes OCE. Add a comment explaining the intentional exclusion. - Add RecordingTimeProvider that captures the dueTime passed to CreateTimer, and two new tests: one asserting all delays are in [0, MaxDelay] across three consecutive retries, and one confirming that a BaseDelay larger than MaxDelay never produces a negative delay. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace `new Uri(context.Request.Url, location)` with `Uri.TryCreate`; when the server-supplied Location value is not a valid URI the parse fails silently and the redirect is treated as non-followable, leaving the 3xx response in context.Response for the caller. Previously the raw UriFormatException escaped the pipeline unhandled. Added tests: - 3xx with a malformed Location (invalid IPv6 literal) → no exception, redirect not followed, 3xx returned, transport called once. - Chained A→B→C→200 multi-hop redirect → ends at 200 with 3 transport calls and request URLs progressing A, B, C in order. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The two prior backoff tests used BaseDelay/attempt values too small to distinguish the fixed overflow-safe cap from the old code. Replaced: - Overflow guard test: BaseDelay=700 days (~6e17 ticks); at shift=4 the old `baseTicks << shift` overflows long and wraps negative, so the jitter cap becomes ≤ 0 and Task.Delay is skipped, leaving fewer timer entries than expected. The test now uses MaxRetryAttempts=5 (reaching shift=4) and asserts that all 5 recorded delays are in [0, MaxDelay=30 s], which fails against the old code. - Delay-bound test: BaseDelay=MaxDelay=30 s so the cap equals MaxDelay for every attempt. The assertion `delay <= MaxDelay` is now governed by the cap logic rather than by BaseDelay being naturally small, making it meaningful against an implementation that forgets to apply the cap. Both tests use the existing RecordingTimeProvider so no real time passes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds InstrumentationPolicy at PipelineStage.Diagnostics. Per attempt: starts a client-kind Activity from DexpaceDiagnostics.ActivitySource with OTel HTTP semantic tags (http.request.method, url.full redacted via UrlRedactor, server.address, server.port, http.response.status_code, http.request.resend_count, error.type on exception). Records http.client.request.duration histogram (seconds) and http.client.active_requests UpDownCounter from the shared Meter. Emits zero-alloc source-generated LoggerMessage events at Debug/Warning with the redacted URL; secrets are never logged. Adds Microsoft.Extensions.Logging.Abstractions 9.0.5 to Directory.Packages.props and references it (no Version) in Core. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds the DexpacePipeline static factory. CreateDefault assembles the full default policy set via PipelineBuilder: OperationPolicy, RedirectPolicy, IdempotencyPolicy, ClientIdentityPolicy, RetryPolicy(timeProvider), SetDatePolicy(timeProvider), optional authPolicy, and InstrumentationPolicy(logger) — then calls Build(transport). PipelineBuilder sorts by stage at Build time, so insertion order does not affect the final chain ordering. Tests verify end-to-end request flow (200), retry wiring (503→200, exhaust), redirect wiring (302→200), and optional auth policy injection. InstrumentationPolicyTests and DexpacePipelineTests are placed in the "Instrumentation" xUnit collection to prevent parallel execution and cross-test ActivitySource leakage. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Inject traceparent/tracestate request headers when Activity.IdFormat is W3C, so trace context propagates over any transport without relying on transport auto-injection; injection is skipped when there is no active activity. - Add url.scheme activity tag per OTel HTTP semantic conventions. - Record http.client.request.duration with http.request.method and http.response.status_code (or error.type on failure) tags so the histogram can be sliced by method and outcome; tag http.client.active_requests with http.request.method as well. - Restore context.Activity to its previous value in the finally block instead of unconditionally setting it to null, so an outer operation-level activity is not clobbered. - Add 6 tests covering: traceparent injection (W3C), tracestate injection, no-injection without listener, duration histogram method+status tags, url.scheme tag, and previous-activity restore. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the request/response pipeline spine and the full set of core policies, turning the transport-agnostic SPI into a usable, observable HTTP client.
HttpPipelinePolicy(object-policy model), the immutable re-entrantPipelineRunner, the stagedPipelineBuilder(pillar-validated, type-targeted edits), and theHttpPipelineentry point with a sync bridge.OperationPolicy(overall timeout),RedirectPolicy(3xx method matrix, cross-origin header stripping, HTTPS→HTTP guard, malformed-Locationhandling),RetryPolicy(semantic retry on typed errors/5xx, full-jitter backoff capped atMaxDelay,Retry-After, idempotency + body-replayability gating),IdempotencyPolicy,SetDatePolicy,ClientIdentityPolicy, andInstrumentationPolicy(per-attemptActivitywith OTel tags + W3Ctraceparent/tracestatepropagation, duration/active-request metrics, redacted structured logs).Response.EnsureSuccessAsync— buffers a bounded error body and throwsHttpResponseExceptionsoGetErrorAsync<T>can read the typed error.DexpacePipeline.CreateDefault— assembles the default pipeline.Microsoft.Extensions.Logging.AbstractionstoCore(the first standard-abstraction dependency, per the platform design). No other new dependencies.Every policy went through spec + code-quality review with fix loops — notably
UrlRedactorrelative-URL leakage, the retry backoff overflow + previously-unexercised delay math, malformed redirectLocation, and the missing W3C trace-context propagation.Test plan
dotnet build -c Releaseclean on net8.0 + net10.0 (warnings-as-errors)dotnet format --verify-no-changescleandotnet test -c Releasepasses (198 tests: 181 core, 17 serialization)🤖 Generated with Claude Code