release: v0.1.0 - resiliency middleware, pre-v1 API, performance and test quality - #4
Merged
Conversation
Add initial project setup with README, Makefile, and configuration files
- Add `cache-dependency-path: go.mod` to all `setup-go` steps in the CI workflow to ensure proper caching. - Fix a typo in `.golangci.yml` for the gocritic `commentedOutCode` disabled check.
- Remove `go.sum` from the `git diff --exit-code` command in the `.github/workflows/ci.yml` build job to only verify `go.mod`.
- Comment out the entire `.github/workflows/ci.yml` file to temporarily disable the GitHub Actions CI pipeline.
Fix: CI cache dependency paths and correct golangci configuration typo
Core HTTP client with middleware chain and resiliency patterns: client, options, transport, middleware chain, timeout, retry with backoff strategies, circuit breaker, rate limiting, logging, metrics, error classification, fluent RequestBuilder and object pooling. Includes unit tests, examples and benchmarks. Zero dependencies.
- timeout: cancel the timeout context on Body.Close, not on RoundTrip return, so streaming/chunked bodies aren't aborted mid-read - retry: skip draining/closing the body on the final attempt, since that response is returned to the caller Adds httptest.Server regression tests that read the body after RoundTrip returns.
Split retryRoundTripper.RoundTrip into canRetry, prepareRetry and drainAndClose, lowering cognitive complexity below the linter threshold and removing the //nolint:gocognit directive.
Add MaxHalfOpenRequests (default 1) and SuccessThreshold (default 1) so Half-Open admits a bounded number of concurrent probes and closes only after enough consecutive successes. Previously the mutex was released between allowRequest and recordResult, letting every concurrent request through in Half-Open. Also split recordResult into recordClosedResult/recordHalfOpenResult, order callees before callers, and add concurrency regression tests.
Switch backoff jitter from math/rand/v2 (Go 1.22+) to math/rand so the stdversion warnings go away while go.mod stays at 1.21. Also adopt the min builtin in place of manual max-capping.
Move the package from the httpclient/ subdirectory to the module root and rename it from httpclient to rhttp, dropping the go-httpclient/httpclient import stutter. Updates go.mod, every import and package clause, the sentinel error prefixes (rhttp:), README, Makefile, .golangci.yml, CLAUDE.md and the example. No behavior change: build, race tests and lint pass under the new path. BREAKING CHANGE: import path is now github.com/oswaldom-code/rhttp and the package identifier is rhttp (was .../go-httpclient/httpclient, httpclient).
Uncomment .github/workflows/ci.yml so the test, lint, build and benchmark jobs run on pull requests and pushes to main. The Go 1.21 matrix job now passes since backoff no longer imports math/rand/v2.
RoundTrip mutated req.Body of the caller's request via prepareRetry, violating the http.RoundTripper contract (RoundTrip must not modify the request) and leaking per-attempt changes between attempts. Split into prepareRequest (clones the request per attempt, rewinds the body via GetBody only on retries) and waitBackoff. Adds regression test TestRetry_DoesNotMutateOriginalRequest.
The middleware stored next on a shared struct created outside the closure, so applying one CircuitBreaker value to two chains made them share state and let the second application overwrite the first chain's transport. Move breaker creation into the closure (independent instance per application) and hold next in a per-chain wrapper. Add NewCircuitBreaker for the opposite, intentional case: a breaker whose state is shared across chains via Middleware().
Replace the fragile strings.Contains fallback in classifyError with structured checks: syscall.ECONNREFUSED/ECONNRESET/EHOSTUNREACH/ ENETUNREACH for connection errors and x509 error types for TLS. Split into classifyTLS and classifyConnection helpers. Remove ErrKindTemporary, which no branch produced and whose retryable semantics IsRetryable already covers. Rewrite the tests that relied on plain-string errors to use real typed errors, and add TestClassify_AllKindsAreReachable to fail if any ErrorKind becomes orphaned.
DefaultIsRetryable retried on any non-nil error, so TLS verification failures and caller cancellations burned every attempt and hid the real error. Route error decisions through Classify(err).Kind.IsRetryable() so only transient kinds (timeout, connection, DNS) retry; TLS and canceled do not. Add TestRetry_RespectsErrorClassification table and migrate the existing retry tests off plain-string errors onto typed ones.
DefaultIsFailure treated any non-nil error as an upstream failure, so a burst of caller cancellations (frontend navigation, shutdown) could open the circuit against a healthy upstream. Classify the error and skip ErrKindCanceled; timeouts still count as failures since a slow upstream is a degraded one. Add TestCircuitBreaker_ClientCancellationsDoNotOpenCircuit and TestCircuitBreaker_TimeoutsOpenCircuit.
drainAndClose read the entire error body before each retry so a large 5xx payload added latency and bandwidth at the worst moment. Cap the drain with io.LimitReader(maxDrainBytes); bodies past the limit leave the connection to be closed instead of reused. Add TestRetry_DrainIsBounded.
Metrics emitted req.URL.Path raw. When a recorder exports Path as a metrics label, per-ID REST paths create one time series per request and grow Prometheus memory without bound. Path is now empty unless a PathNormalizer is provided, deferring the cardinality decision to the caller. Provide func(p string) string that collapses high-cardinality segments to a template. Diagnostic 4.3.
…mbers The 35% faster claim compared wrapper overhead over a no-op transport, not real network requests, and read as a network-performance claim the benchmarks do not support. Rename BenchmarkClient_* to BenchmarkMiddlewareOverhead_* to reflect what they measure, rewrite the Benchmarks section with an explicit methodology note, and run make bench with -count=5 for reproducibility. Diagnostic 2.1.
The 100% test coverage - 101 tests line is a claim that expires on every PR. Defer to the dynamic Codecov badge as the single source of truth for coverage. Diagnostic 2.2.
The chain applies the first middleware as the outermost wrapper, and Timeout placement relative to Retry silently selects total-budget vs per-attempt timeout semantics - the kind of undocumented detail that causes incidents. Add a Middleware Order section covering the outermost-first rule, both timeout patterns, and the Retry/CircuitBreaker interaction, plus runnable ExampleRetry_totalBudget and ExampleRetry_perAttemptTimeout. Diagnostic 4.4.
Align the recommended middleware order with the diagnostic: Retry now sits outside CircuitBreaker (... RateLimit -> Retry -> CircuitBreaker) so every attempt consults the circuit and a tripped breaker short-circuits the remaining attempts. Updated doc.go package godoc, CLAUDE.md, and the README order line, example block, and interaction table.
New exported symbols shipped without godoc and rendered bare on pkg.go.dev. Add doc comments to SharedCircuitBreaker, NewCircuitBreaker, Middleware, State and the MetricsConfig.PathNormalizer field. Document SetBody buffering: bodies up to 10 MB are buffered so retries can rewind them; larger bodies stream and are sent once without retry. Without this note the >10 MB no-retry case is a silent surprise.
The Motivation and Usage Modes sections were the last Spanish prose in an otherwise English README. Unify the language for general adoption. Diagnostic 5.2 (T3).
added 25 commits
July 25, 2026 11:32
Timeout(0) wrapped every request in an already-expired context, so all requests failed with deadline exceeded (and a negative duration did the same). Follow the project convention that an invalid config returns the next RoundTripper unchanged.
A non-positive rate produced a 1/rate wait time (division by zero or negative), busy-looping WaitContext at 100% CPU (forever via the deprecated Wait()). A burst below 1 left maxTokens at 0 so TryAcquire never succeeded, blocking forever. Validate in the constructor and return an unlimited bucket that allows every request, matching the invalid-config-is-a-no-op convention used by Timeout and RateLimit.
recordResult held cb.mu while calling the user-supplied IsFailure callback. A callback that inspected the breaker (e.g. State(), which locks cb.mu) deadlocked, since sync.Mutex is not reentrant. Evaluate IsFailure before acquiring the lock and pass the result into the state machine.
A request admitted in one state could record its result after the breaker had transitioned, corrupting the current episode: a slow Closed request completing during Half-Open ran recordHalfOpenResult, closing the circuit and freeing the real probe's budget. Track a generation that bumps on every state transition; allowRequest returns the admission generation and recordResult discards results whose generation no longer matches. This also makes the Open case in recordResult genuinely unreachable (C13), with an accurate comment.
New now returns *Client, R() is a method on it, and the type-assert fallback in the old free R() is gone. Pre-v1 breaking change per plan A1.
The interface is now TryAcquire plus WaitContext, so an adapter over an x/time/rate style limiter fits in two one-line methods. The deprecated TokenBucket.Wait concrete method is deleted pre-v1 per plan A2.
The adapter now lives in middleware.go with a godoc example showing a five-line custom middleware. All tests migrate off internal.RoundTripperFunc, which leaves the internal package empty, so it is deleted. Plan A4.
bench_test.go moves to the exported *rhttp.Client after A1. REPORT.md is the 2026-07-25 re-run: allocations unchanged (15 overhead, 79 E2E), rhttp still the cheapest wrapper at 1.00x overhead.
The type did not satisfy RateLimiter and could not plug into the RateLimit middleware, so it was an exported dead end. Deleted per plan A5; it can return post-v1 together with a dedicated per-host middleware.
BackoffFunc gains a resp parameter carrying the response of the attempt that triggered the retry (nil when it produced none). The seven strategies ignore it and keep their pure attempt-to-duration logic at zero allocs (verified against a 5-sample baseline). RetryConfig.Backoff is now typed as BackoffFunc and the retry loop threads the previous response through. New WithRetryAfter decorator waits max(base, server hint) on 429/503, parsing both delay-seconds and HTTP-date formats. Plan A3.
DecodeJSON always drains and closes the body, treats status >= 300 as an error without decoding, and streams the decode on 2xx. CircuitState gains a lowercase String method. Plan A6+A7; RetryConfig.Backoff typing already landed with A3 and the builder one-shot godoc belongs to C10.
A RoundTripper owns the request body even when it fails the request. Circuit-open, rate-limited, canceled Retry-After waits, and canceled or unpreparable retry attempts now release it via closeRequestBody. Plan C6.
base shifted by attempts around 37 wrapped negative, producing negative waits that time.After treats as zero and disabling backoff entirely. The three exponential variants now saturate at maxDuration once the product no longer fits in int64. Plan C7.
Do(nil, req) panicked in req.Clone; a nil ctx now falls back to context.Background. The three select waits (retry backoff, token refill, Retry-After period) switch from time.After to a stopped time.Timer so a canceled context releases the timer immediately. Plan C8+C9.
resolveBody handed back the same bytes.Reader on every execute, so a second run sent an empty body with a mismatched ContentLength; it now builds a fresh reader from bodyBytes. The header copy loop becomes a direct assignment (Do clones the request, sharing is safe), and the builder's one-shot, single-goroutine contract is documented. Plan C10+P3.
Do already clones the request before it enters the chain, so cloning again for attempt 0 only protected retries, which build their own clone. MiddlewareOverhead_WithRetry drops 7 to 4 allocs/op (456 to 250 ns/op) and AllMiddleware 16 to 13 allocs/op. Plan P1.
TokenBucket computes its per-token refill wait once in the constructor instead of under the mutex on every WaitContext iteration. RequestBuilder allocates the query and path parameter maps on first use, and the DecorrelatedJitterBackoff godoc now states that concurrent sequences sharing one instance correlate their delays. Plan C11+P4.
The two no-retry guards now fail with syscall.ECONNREFUSED, a genuinely retryable error, so they prove the guard and not the error class. Basic auth asserts the exact encoded header, the builder timeout uses errors.Is with context.DeadlineExceeded, and the failure-count reset test gains its control case: three straight failures do open. Plan Q1.
Timeout outside Retry enforces a total budget and cuts the run mid backoff; Retry outside Timeout gives every attempt a fresh deadline; and a breaker tripped mid-retry short-circuits the remaining budget because ErrCircuitOpen is not retryable. Plan Q2.
A body one byte over the 10 MB cap must stream: GetBody stays nil, the transport receives every byte exactly once, and a retryable 503 does not trigger a second attempt because the reader cannot be replayed. Plan Q3.
SharedCircuitBreaker.State is asserted through a full closed-open-half- open-closed cycle, observing half-open deterministically with a probe held in flight. TokenBucket.Tokens gets its first coverage. The reset timeout sleeps move from 5ms of slack to at least 50ms to survive loaded CI runners. Plan Q4.
Rate limiter: WaitContext on a pre-canceled ctx, HTTP-date Retry-After, ctx cutting a Retry-After wait short, and 50 goroutines racing the RespectRetryAfter bookkeeping. Builder: malformed URL, JSON and XML marshal error branches, XML happy path, custom Execute method, and path parameter escaping. Plan Q5.
coverage-summary now regenerates the profile instead of reading whatever coverage.out was left on disk (a stale one from the old module path made it fail), the ci target gets the deps rule it referenced but never had, and .PHONY covers every declared target.
The README Benchmarks section now carries the 2026-07-25 post- optimization numbers: 13 allocs for the full stack, 910ns/12 allocs as a wrapper against equivalently configured competitors (net/http floor at 1.92x), the loopback E2E table, and the honesty caveats that qualify both. REPORT.md is the regenerated canonical snapshot. Plan R1.
oswaldom-code
changed the title
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.
Release v0.1.0: all five phases of the development plan are complete.
Summary
Benchmarks (2026-07-25)
Full middleware stack: ~1 us, 13 allocs/op. As a wrapper (equivalent config), 910 ns/12 allocs vs net/http floor at 1.92x. See the README Benchmarks section and benchmarks/REPORT.md for tables and caveats.
Verification