Phase 1: correctness fixes, circuit-breaker hardening, and rename to rhttp - #3
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.
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
Closes out Phase 1 of the resilient HTTP client: fixes the three known correctness/design bugs, hardens the circuit breaker, keeps the Go 1.21 floor, and renames the module to drop the import stutter. Fast-forward merge (no conflicts).
Highlights
🐛 Correctness fixes (with regression tests)
Body.Close()instead of onRoundTripreturn, so streaming/chunked bodies are no longer aborted mid-read (context canceled).MaxHalfOpenRequests(default 1 → true single-probe) andSuccessThreshold(default 1). Fixes the previously incorrect "only one at a time due to mutex" behaviour.♻️ Refactors
retryRoundTripper.RoundTripintocanRetry/prepareRetry/drainAndClose, removing the//nolint:gocognit.recordResultintorecordClosedResult/recordHalfOpenResult(flat, early-returns) and made the state switch exhaustive.🔧 Compatibility
math/rand/v2→math/randso thego 1.21floor ingo.modis honoured (no consumer on 1.21 breaks). Adopt theminbuiltin.📦 Rename (BREAKING)
github.com/oswaldom-code/go-httpclient→github.com/oswaldom-code/rhttp; package moved from thehttpclient/subdirectory to the module root and renamedhttpclient→rhttp. Sentinel error prefixes are nowrhttp:.⚙️ CI
.github/workflows/ci.yml(test matrix 1.21/1.22/1.23, lint, build, benchmark).Verification
go build ./...,go test ./... -race,golangci-lint run ./...,gofmt— all clean locally.