Add streaming support, request cancellation, and client hardening#16
Conversation
Streaming (new feature): - Add ISimpleStreamResponse (IDisposable) and SimpleStreamResponse exposing the raw, unbuffered response Stream alongside StatusCode/IsSuccessful/Headers. - Add ISimpleClient.MakeStreamRequest, which sends with HttpCompletionOption.ResponseHeadersRead and hands the caller-owned HttpResponseMessage to the response so the connection stays open until disposed. Request cancellation: - Add an optional CancellationToken to MakeRequest / MakeRequest<T>. A caller cancellation now surfaces as OperationCanceledException, while a timeout still surfaces as TimeoutException (shared SendHttpRequest helper). HttpClient lifetime / thread-safety: - Guard lazy creation and periodic replacement of the HttpClient with a lock so concurrent first-requests can't create duplicate clients/timers. - Dispose retired (self-created) HttpClients after a grace period instead of leaking them; factory-created clients are left for the factory to manage. Header handling: - Apply request headers with TryAddWithoutValidation and route content-level headers to the body content, so setting headers like Content-Disposition (or otherwise "invalid" values) no longer throws. - Populate response headers via the indexer to avoid throwing if a header name appears in both the response and content header collections. Tests cover the streaming surface, request cancellation, and custom/content-level request headers. All 79 tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
New backward-compatible features (streaming requests, request cancellation) plus client hardening fixes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
net6.0 is out of support; net10.0 is the current LTS. Removes the need for the DOTNET_ROLL_FORWARD workaround when running tests locally. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Introduce ISimpleClientFactory.CreateClient(host) as the preferred DI entry point: each consumer gets its own client with its own host, instead of sharing a single client and mutating its Host property (which collides when consumers share a scope). - Register ISimpleClientFactory as a singleton in AddSimpleHttpClient. - Change the ISimpleClient registration from scoped to transient so direct injection also gives each consumer an independent instance. - Document the factory as the recommended approach in the README. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a net8.0 target alongside netstandard2.0. On the modern target the handler is a SocketsHttpHandler with PooledConnectionLifetime, which keeps DNS fresh by rotating pooled connections - so the periodic HttpClient replacement timer, retired-client grace disposal, and .NET Framework runtime check are all compiled out there (guarded by #if NETSTANDARD2_0). The netstandard2.0 build keeps the existing rotation behavior since it can't reference SocketsHttpHandler. Also: - Rewrite the retired-client disposal (netstandard2.0 only) from Task.Delay(...).ContinueWith(...) to an async/await helper, avoiding the TaskScheduler.Current capture. - Bump Microsoft.Extensions.Http to 8.0.1 and Newtonsoft.Json to 13.0.3. - Document cancellationToken params and supported frameworks. - Version 4.3.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
WireMock.Net 1.5.22 pulled in transitively vulnerable packages (NU1902/NU1903 via Microsoft.IdentityModel.* and System.Linq.Dynamic.Core). 2.7.0 clears them. Align the test project's Microsoft.Extensions.DependencyInjection to 10.0.0 to satisfy the updated transitive graph. Test-only change; nothing ships in the package. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- ci.yml: build the multi-targeted library and run tests on pushes to main and on pull requests. - release.yml: on a published GitHub Release, run tests, build the package with the version taken from the release tag, and push it to NuGet. This removes the need to edit <Version> manually - the release tag is the version. Requires a NUGET_API_KEY repository secret. - Add a CI badge to the README. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Timeout_WaitsTheCorrectAmountOfTime and TimeoutOverride_OverridesClientTimeout pointed a request at a dead localhost path and assumed the connection would hang until the timeout. On Linux (CI) that's an instant connection-refused, which throws HttpRequestException instead of timing out, so the tests failed there. Use a WireMock endpoint that delays well past the timeout so the timeout reliably fires on any platform. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move all the HttpClient acquisition/rotation/disposal logic out of SimpleClient and behind an internal IHttpClientProvider seam, so SimpleClient is free of conditional compilation: - PooledHttpClientProvider (net8.0): single SocketsHttpHandler-backed client (or factory.CreateClient per call); no rotation needed. - RotatingHttpClientProvider (netstandard2.0): timer-based replacement, retired-client grace disposal, and .NET Framework detection. - HttpClientProviderFactory: the single #if that selects the implementation. SimpleClient now just holds an IHttpClientProvider and calls GetClient(). Both target frameworks build and all 78 tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Multi-target the test project to net10.0;net48. net10 exercises the library's net8.0 asset (modern SocketsHttpHandler path); net48 exercises the netstandard2.0 asset (rotating HttpClient path + .NET Framework branch) that .NET Framework consumers receive. Split CI into an ubuntu job (net10) and a windows job (net48). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Publishing now happens either via manual workflow_dispatch (choosing patch/minor/major) or automatically when a PR is merged to main with a release:major|minor|patch label; unlabeled merges are skipped. The version is computed by bumping the latest v* git tag (falling back to the csproj <Version> when no tag exists), and each publish creates the matching tag and a GitHub Release - so <Version> never needs manual editing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The net48 target was intended in an earlier commit but the edit didn't apply, so the windows CI job failed with NETSDK1005 (no net48 target). Add TargetFrameworks=net10.0;net48, LangVersion=latest (for C# 8+ features on net48), and the framework System.Net.Http reference net48 SDK projects require. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- .NET Framework doesn't get the implicit System.Net.Http global using that .NET Core does, so add it for net48 (fixes CS0246 on HttpMethod in the tests). - Mark the windows net48 job continue-on-error so any remaining net48-specific quirks surface without blocking the build while the path is being validated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
HttpMethod.Patch doesn't exist on .NET Framework 4.8 (added in netstandard2.1); constructing the method explicitly works on every target. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two failures on net48 were genuine .NET Framework behavior differences, not bugs: - GET-with-body throws ProtocolViolationException on .NET Framework's HttpClient, so Get_Request_WithBody_Succeeds is guarded with #if !NETFRAMEWORK (and the limitation is documented in the README). - .NET Framework's XmlSerializer declares xmlns:xsd before xmlns:xsi; the exact-string assertion in Serialization_Succeeds is now framework-aware. With both frameworks green (net10.0: 78, net48: 77), drop continue-on-error so the windows net48 job is a real CI gate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
.NET Framework's HttpClient throws a raw ProtocolViolationException when a request body is sent with a verb that disallows it (most commonly a GET with a body). Catch it in SendHttpRequest and rethrow as a NotSupportedException with an actionable message. GET-with-body still works as before on modern runtimes. Replace the net48-skipped test with one that asserts the NotSupportedException, so the limitation is verified rather than just compiled out; modern runtimes keep the success test. Both frameworks: 78 tests passing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…d response once The IHttpClientProvider files were added earlier but SimpleClient still contained the inline HttpClient replacement logic - so the providers were dead code. Wire SimpleClient to hold an IHttpClientProvider (via HttpClientProviderFactory) and call GetClient(), removing the inline timer/replacement/RuntimeInformation and the Dispose timer handling. All client-lifetime logic now lives in the providers. Also in SimpleClient (same file, so same commit): - The request reflects what was sent: an object Body is the source of truth, serialized on every send (so re-sending after changing Body sends the new value) with the serialized form reflected onto StringBody; a string body is sent as-is; ContentType resolution moved into AddRequestBody and is reflected onto request.ContentType. - The response body is read once (bytes, then decoded with the response charset, BOM-aware) instead of ReadAsStringAsync + ReadAsByteArrayAsync. Tests: object Body now takes precedence over a directly-set StringBody, plus a resend regression test. net10.0 and net48: 79 each. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Add HttpClientConfigurator.GetConfiguredHttpClient() (handler + new client + configure in one call); both providers use it instead of repeating those three steps. GetMessageHandler and ConfigureHttpClient remain for the IHttpClientFactory registration. - Refactor RotatingHttpClientProvider.GetClient around a single hasFactory flag with an early return for the factory-on-modern case (no locking there), and share the factory-vs-own creation in one CreateClient(useFactory) helper used by both GetClient and ReplaceClient. The .NET Framework check is now an IsDotNetFramework property. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
21a1ae0 to
c41af27
Compare
Mako88
left a comment
There was a problem hiding this comment.
Thanks for the thorough PR — the broad direction looks good, and CI is green on the current head.
I found one thing I’d like cleaned up before approval: the streaming cancellation docs/API contract currently imply the CancellationToken passed to MakeStreamRequest cancels later reads from the returned stream. In practice, that token is only used for SendAsync through response headers; after MakeStreamRequest returns a raw Stream, subsequent ReadAsync calls won’t automatically observe the original token. Callers need to pass their token into the stream read operations themselves.
Suggested fix: update the README/XML docs wording to say the token cancels sending the request and waiting for response headers. For reading, callers should use the ReadAsync(..., cancellationToken) overloads where available, or otherwise close/dispose the response/stream to abort the in-flight read.
Everything else I reviewed looks reasonable: the ResponseHeadersRead lifetime is tied to disposing SimpleStreamResponse, headers are now applied after content so content headers can land on HttpContent, duplicate response/content headers are handled via index assignment, and the new provider split makes the client lifetime behavior easier to reason about.
… serializer Wrap the streaming response body in CancellationAwareStream so the token passed to MakeStreamRequest is observed by reads, not just by sending the request. The token is baked into the stream, so even readers with no token parameter (e.g. StreamReader.ReadLineAsync) honor cancellation through the reads they make internally. Async reads link the token for clean OperationCanceledException even mid-read; sync reads observe it between reads. Docs (interface, impl, README) updated to the now-true contract. Add SimpleHttpSystemTextJsonSerializer as an opt-in serializer alongside the Newtonsoft-based default, with settings mirroring it (camelCase, null values omitted, indented, case-insensitive deserialize) and a shared cached options instance. Newtonsoft remains the default; it becomes the default and Newtonsoft is removed in v5. Bump version to 4.2.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Summary
Adds streaming response support and request cancellation, plus several hardening fixes to the HTTP client, and expands the README.
New features
MakeStreamRequestreturns anISimpleStreamResponse(IDisposable) exposing the raw, unbuffered responseStreamalongsideStatusCode/IsSuccessful/Headers. Sends withHttpCompletionOption.ResponseHeadersRead; the caller owns the connection lifetime via disposal.CancellationTokenadded toMakeRequest/MakeRequest<T>. Caller cancellation surfaces asOperationCanceledException; timeouts still surface asTimeoutException.Hardening fixes
TryAddWithoutValidationand route content-level headers (e.g.Content-Disposition) to the body content, so they no longer throw.Docs
ISimpleHttpClientreference (the registered interface isISimpleClient).Tests
All 74 integration tests pass, including new coverage for the streaming surface, request cancellation, and custom/content-level request headers.
Notes for the maintainer
request.StringBody/ContentTypemutation (covered by existing tests) and the double read of the response body (operates on already-buffered content; reworking risks charset/BOM regressions). Happy to revisit either in a separate PR.🤖 Generated with Claude Code
Solves #3