diff --git a/CHANGELOG.md b/CHANGELOG.md index fe23078..9796ea4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,24 @@ All notable changes to this project are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.2.0] - 2026-08-05 + +### Added + +- `ErrKindDNSNotFound` error kind and the `IsDNSNotFound` helper, for a name that does not exist (NXDOMAIN). The kind is appended last in the `ErrorKind` block, so the numeric values shipped in 0.1.0 are unchanged. + +### Fixed + +- `DefaultIsRetryable` no longer retries permanent DNS failures. `classifyError` now consults `net.DNSError.IsNotFound`: an NXDOMAIN classifies as the non-retryable `ErrKindDNSNotFound`, while a transient resolution failure stays `ErrKindDNS` and stays retryable. A misspelled or decommissioned hostname previously consumed the whole attempt budget plus the full backoff schedule on an outcome that could never succeed. + +### Changed + +- `IsDNS` now reports true for both `ErrKindDNS` and `ErrKindDNSNotFound`: an NXDOMAIN is still a DNS failure. Callers that need only the permanent case should use `IsDNSNotFound`. + +### Performance + +- The `Timeout` middleware attaches its context with `req.WithContext` instead of `req.Clone`. The deep copy was redundant — `Do` already clones the caller's request before the chain runs — and cost a duplicated struct, URL and header map on every request. The full middleware stack drops from 13 to 11 allocations and ~1589 to ~1304 B/op. + ## [0.1.0] - 2026-07-25 First public release. @@ -21,4 +39,5 @@ First public release. - Error classification: `Classify`, `IsRetryable`, `IsTimeout`, `IsConnection`, and related helpers. - Zero external dependencies; Go standard library only. +[0.2.0]: https://github.com/oswaldom-code/rhttp/releases/tag/v0.2.0 [0.1.0]: https://github.com/oswaldom-code/rhttp/releases/tag/v0.1.0 diff --git a/README.md b/README.md index f652cd9..0d59541 100644 --- a/README.md +++ b/README.md @@ -275,20 +275,26 @@ if err != nil { case rhttp.ErrKindConnection: // Connection refused, reset, etc. case rhttp.ErrKindDNS: - // DNS resolution failed + // DNS resolution failed, transiently + case rhttp.ErrKindDNSNotFound: + // NXDOMAIN: the name does not exist. Permanent, never retried case rhttp.ErrKindTLS: // Certificate error - case rhttp.ErrKindTemporary: - // Temporary error, may resolve on retry } // Or use helpers if rhttp.IsRetryable(err) { - // Safe to retry (timeout, connection, DNS, temporary) + // Safe to retry (timeout, connection, transient DNS) } } ``` +`ErrKindDNS` and `ErrKindDNSNotFound` are split because they call for opposite +handling: a SERVFAIL may clear on the next lookup, while an NXDOMAIN cannot — +retrying it only spends the attempt budget and the full backoff schedule on an +outcome that is already decided. `IsDNS` matches both; `IsDNSNotFound` singles +out the permanent one. + ## Middleware Order The **first middleware in the list is the outermost**: it runs first on the way in and last on the way out. Each subsequent middleware wraps the ones after it, and the transport sits at the center. @@ -344,7 +350,7 @@ transport := rhttp.DefaultTransport() // HTTP/2 enabled, optimized pool ## Benchmarks -Two suites, measured 2026-07-25 on linux/amd64 (Intel Core i7-1255U, Go 1.24): +Two suites, measured 2026-08-05 on linux/amd64 (Intel Core i7-1255U, Go 1.24.1): the in-repo microbenchmarks (`make bench`) measure client and middleware overhead against a no-op transport, and a standalone comparison harness ([`benchmarks/`](benchmarks/), `make report`) measures rhttp against Resty @@ -359,7 +365,7 @@ Minimum of 5 runs: BenchmarkMiddlewareOverhead_Baseline-12 240 ns/op 656 B/op 4 allocs/op BenchmarkMiddlewareOverhead_WithRetry-12 272 ns/op 656 B/op 4 allocs/op BenchmarkMiddlewareOverhead_WithCircuitBreaker-12 266 ns/op 656 B/op 4 allocs/op -BenchmarkMiddlewareOverhead_AllMiddleware-12 1030 ns/op 1589 B/op 13 allocs/op +BenchmarkMiddlewareOverhead_AllMiddleware-12 1008 ns/op 1304 B/op 11 allocs/op BenchmarkStdHttpClient_Baseline-12 256 ns/op 552 B/op 5 allocs/op BenchmarkTokenBucket_TryAcquire-12 48 ns/op 0 B/op 0 allocs/op BenchmarkBackoffStrategies/Exponential-12 8 ns/op 0 B/op 0 allocs/op @@ -375,23 +381,23 @@ Wrapper overhead (no-op transport, timeout + 3-attempt retry configured everywhe | Client | ns/op | allocs/op | vs best | |---|---:|---:|---:| -| rhttp (Timeout+Retry) | 910 | 12 | 1.00x | -| rhttp (Timeout+Retry+CircuitBreaker) | 946 | 12 | 1.04x | -| net/http (Timeout only, no retry) | 1750 | 26 | 1.92x | -| go-retryablehttp | 1775 | 26 | 1.95x | -| Heimdall (retry) | 2555 | 32 | 2.81x | -| Resty (retry) | 5818 | 48 | 6.39x | +| rhttp (Timeout+Retry) | 784 | 10 | 1.00x | +| rhttp (Timeout+Retry+CircuitBreaker) | 785 | 10 | 1.00x | +| net/http (Timeout only, no retry) | 1672 | 26 | 2.13x | +| go-retryablehttp | 1719 | 26 | 2.19x | +| Heimdall (retry) | 2579 | 32 | 3.29x | +| Resty (retry) | 5803 | 48 | 7.40x | End-to-end (~1 KB JSON over loopback): | Client | ns/op | allocs/op | vs best | |---|---:|---:|---:| -| go-retryablehttp | 58309 | 74 | 1.00x | -| net/http (Timeout only, no retry) | 60995 | 75 | 1.05x | -| Heimdall (retry) | 61009 | 80 | 1.05x | -| rhttp (Timeout+Retry) | 61923 | 76 | 1.06x | -| rhttp (Timeout+Retry+CircuitBreaker) | 62817 | 76 | 1.08x | -| Resty (retry) | 71696 | 96 | 1.23x | +| go-retryablehttp | 60326 | 74 | 1.00x | +| net/http (Timeout only, no retry) | 61578 | 75 | 1.02x | +| Heimdall (retry) | 64415 | 80 | 1.07x | +| rhttp (Timeout+Retry+CircuitBreaker) | 64939 | 74 | 1.08x | +| rhttp (Timeout+Retry) | 66714 | 74 | 1.11x | +| Resty (retry) | 73722 | 96 | 1.22x | **Read the caveats before quoting these numbers:** @@ -467,7 +473,7 @@ All PRs must pass CI checks before merging. ## Roadmap -> **Status:** v0.1.0 released (Phase 1 complete). Phase 2 is the next focus. +> **Status:** v0.2.0 released (Phase 1 complete). Phase 2 is the next focus. ### Phase 1: Foundation (Completed) diff --git a/benchmarks/REPORT.md b/benchmarks/REPORT.md index 185a1b4..0319a75 100644 --- a/benchmarks/REPORT.md +++ b/benchmarks/REPORT.md @@ -1,6 +1,6 @@ # HTTP client comparison report -Generated: 2026-07-25 13:04 CEST +Generated: 2026-08-05 17:18 CEST ## Environment @@ -31,23 +31,23 @@ Caveats: net/http does not retry (it is the floor, not a symmetric competitor); | Client | ns/op (min) | ns/op (mean) | B/op | allocs/op | vs best | |---|---:|---:|---:|---:|---:| -| rhttp (Timeout+Retry+CircuitBreaker) | 1012 | 1058 | 1468 | 12 | 1.00x | -| rhttp (Timeout+Retry) | 1019 | 1099 | 1468 | 12 | 1.01x | -| net/http (Timeout only, no retry) | 1784 | 1854 | 1594 | 26 | 1.76x | -| go-retryablehttp | 1909 | 1965 | 1595 | 26 | 1.89x | -| Heimdall (retry) | 2797 | 2969 | 2221 | 32 | 2.76x | -| Resty (retry) | 6486 | 6787 | 4885 | 48 | 6.41x | +| rhttp (Timeout+Retry) | 784 | 827 | 1275 | 10 | 1.00x | +| rhttp (Timeout+Retry+CircuitBreaker) | 785 | 860 | 1275 | 10 | 1.00x | +| net/http (Timeout only, no retry) | 1672 | 1726 | 1594 | 26 | 2.13x | +| go-retryablehttp | 1719 | 1810 | 1595 | 26 | 2.19x | +| Heimdall (retry) | 2579 | 2694 | 2221 | 32 | 3.29x | +| Resty (retry) | 5803 | 6056 | 4885 | 48 | 7.40x | ## Results: end-to-end (loopback, ~1 KB JSON) | Client | ns/op (min) | ns/op (mean) | B/op | allocs/op | vs best | |---|---:|---:|---:|---:|---:| -| go-retryablehttp | 59892 | 63859 | 6357 | 74 | 1.00x | -| Heimdall (retry) | 61990 | 67502 | 6999 | 80 | 1.04x | -| net/http (Timeout only, no retry) | 62209 | 67362 | 6584 | 75 | 1.04x | -| rhttp (Timeout+Retry) | 63382 | 66040 | 7224 | 76 | 1.06x | -| rhttp (Timeout+Retry+CircuitBreaker) | 66125 | 67722 | 7185 | 76 | 1.10x | -| Resty (retry) | 72794 | 78349 | 10916 | 96 | 1.22x | +| go-retryablehttp | 60326 | 62835 | 6347 | 74 | 1.00x | +| net/http (Timeout only, no retry) | 61578 | 65103 | 6573 | 75 | 1.02x | +| Heimdall (retry) | 64415 | 66765 | 6973 | 80 | 1.07x | +| rhttp (Timeout+Retry+CircuitBreaker) | 64939 | 68159 | 7034 | 74 | 1.08x | +| rhttp (Timeout+Retry) | 66714 | 69344 | 7023 | 74 | 1.11x | +| Resty (retry) | 73722 | 78923 | 10979 | 96 | 1.22x | ## Reproduce diff --git a/client_test.go b/client_test.go index af627b2..c6e0894 100644 --- a/client_test.go +++ b/client_test.go @@ -4,6 +4,7 @@ import ( "context" "net/http" "testing" + "time" "github.com/oswaldom-code/rhttp" ) @@ -38,6 +39,36 @@ func TestClient_Do_NilRequest(t *testing.T) { } } +func TestClient_Do_ShieldsCallerRequestFromMiddleware(t *testing.T) { + setHeader := func(next http.RoundTripper) http.RoundTripper { + return rhttp.RoundTripperFunc(func(req *http.Request) (*http.Response, error) { + req.Header.Set("X-Request-ID", "generated-in-chain") + return next.RoundTrip(req) + }) + } + + rt := rhttp.RoundTripperFunc(func(req *http.Request) (*http.Response, error) { + if req.Header.Get("X-Request-ID") == "" { + t.Error("expected the middleware header to reach the transport") + } + return &http.Response{StatusCode: http.StatusOK, Request: req}, nil + }) + + c := rhttp.New( + rhttp.WithTransport(rt), + rhttp.WithMiddleware(setHeader, rhttp.Timeout(5*time.Second)), + ) + + req, _ := http.NewRequest(http.MethodGet, "http://example.com", http.NoBody) + if _, err := c.Do(context.Background(), req); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if got := req.Header.Get("X-Request-ID"); got != "" { + t.Fatalf("Do leaked a middleware header onto the caller's request: %q", got) + } +} + func TestClient_MiddlewareChain(t *testing.T) { var order []int diff --git a/errorclass.go b/errorclass.go index e9dc64b..04e5237 100644 --- a/errorclass.go +++ b/errorclass.go @@ -26,11 +26,15 @@ const ( // ErrKindConnection indicates a connection error (refused, reset, etc.). ErrKindConnection - // ErrKindDNS indicates a DNS resolution failure. + // ErrKindDNS indicates a transient DNS resolution failure. ErrKindDNS // ErrKindTLS indicates a TLS/SSL error. ErrKindTLS + + // ErrKindDNSNotFound indicates the name does not exist (NXDOMAIN). Unlike + // ErrKindDNS this is permanent: retrying the same name cannot succeed. + ErrKindDNSNotFound ) // String returns a human-readable name for the error kind. @@ -44,6 +48,8 @@ func (k ErrorKind) String() string { return "connection" case ErrKindDNS: return "dns" + case ErrKindDNSNotFound: + return "dns_not_found" case ErrKindTLS: return "tls" default: @@ -164,6 +170,9 @@ func classifyError(err error) ErrorKind { var dnsErr *net.DNSError if errors.As(err, &dnsErr) { + if dnsErr.IsNotFound { + return ErrKindDNSNotFound + } return ErrKindDNS } @@ -201,13 +210,24 @@ func IsConnection(err error) bool { return classified.Kind == ErrKindConnection } -// IsDNS returns true if the error is a DNS error. +// IsDNS returns true if the error is a DNS error, whether transient or +// permanent. Use IsDNSNotFound to single out the permanent case. func IsDNS(err error) bool { if err == nil { return false } classified := Classify(err) - return classified.Kind == ErrKindDNS + return classified.Kind == ErrKindDNS || classified.Kind == ErrKindDNSNotFound +} + +// IsDNSNotFound returns true if the name does not exist (NXDOMAIN). Such an +// error is permanent and is never retried by DefaultIsRetryable. +func IsDNSNotFound(err error) bool { + if err == nil { + return false + } + classified := Classify(err) + return classified.Kind == ErrKindDNSNotFound } // IsTLS returns true if the error is a TLS error. diff --git a/errorclass_test.go b/errorclass_test.go index 417ffaa..29fdc6f 100644 --- a/errorclass_test.go +++ b/errorclass_test.go @@ -43,6 +43,60 @@ func TestClassify_DNSError(t *testing.T) { } } +func TestClassify_DNSNotFound(t *testing.T) { + dnsErr := &net.DNSError{ + Err: "no such host", + Name: "this-host-does-not-exist.invalid", + IsNotFound: true, + } + + classified := rhttp.Classify(dnsErr) + if classified.Kind != rhttp.ErrKindDNSNotFound { + t.Errorf("expected ErrKindDNSNotFound, got %v", classified.Kind) + } + if classified.Kind.IsRetryable() { + t.Error("NXDOMAIN is permanent: expected IsRetryable to be false") + } + if rhttp.IsRetryable(dnsErr) { + t.Error("expected package-level IsRetryable to be false for NXDOMAIN") + } + if rhttp.DefaultIsRetryable(nil, dnsErr) { + t.Error("expected DefaultIsRetryable to be false for NXDOMAIN") + } +} + +func TestClassify_DNSNotFound_WrappedInURLError(t *testing.T) { + err := &url.Error{ + Op: "Get", + URL: "http://this-host-does-not-exist.invalid/", + Err: &net.OpError{ + Op: "dial", + Net: "tcp", + Err: &net.DNSError{Err: "no such host", IsNotFound: true}, + }, + } + + if got := rhttp.Classify(err).Kind; got != rhttp.ErrKindDNSNotFound { + t.Errorf("expected ErrKindDNSNotFound through the real error chain, got %v", got) + } +} + +func TestClassify_DNSTemporaryStaysRetryable(t *testing.T) { + dnsErr := &net.DNSError{ + Err: "server misbehaving", + Name: "example.com", + IsTemporary: true, + } + + classified := rhttp.Classify(dnsErr) + if classified.Kind != rhttp.ErrKindDNS { + t.Errorf("expected ErrKindDNS, got %v", classified.Kind) + } + if !classified.Kind.IsRetryable() { + t.Error("expected a temporary DNS failure to stay retryable") + } +} + func TestClassify_ConnectionRefused(t *testing.T) { err := &net.OpError{Op: "dial", Net: "tcp", Err: syscall.ECONNREFUSED} classified := rhttp.Classify(err) @@ -156,6 +210,7 @@ func TestErrorKind_String(t *testing.T) { {rhttp.ErrKindCanceled, "canceled"}, {rhttp.ErrKindConnection, "connection"}, {rhttp.ErrKindDNS, "dns"}, + {rhttp.ErrKindDNSNotFound, "dns_not_found"}, {rhttp.ErrKindTLS, "tls"}, {rhttp.ErrKindUnknown, "unknown"}, } @@ -182,6 +237,7 @@ func TestErrorKind_IsRetryable(t *testing.T) { notRetryable := []rhttp.ErrorKind{ rhttp.ErrKindCanceled, rhttp.ErrKindTLS, + rhttp.ErrKindDNSNotFound, rhttp.ErrKindUnknown, } for _, k := range notRetryable { @@ -229,11 +285,33 @@ func TestIsDNS(t *testing.T) { if !rhttp.IsDNS(dnsErr) { t.Error("expected IsDNS to be true for DNSError") } + + notFound := &net.DNSError{Err: "no such host", Name: "invalid.example.com", IsNotFound: true} + if !rhttp.IsDNS(notFound) { + t.Error("expected IsDNS to stay true for NXDOMAIN: it is still a DNS failure") + } + if rhttp.IsDNS(context.Canceled) { t.Error("expected IsDNS to be false for Canceled") } } +func TestIsDNSNotFound(t *testing.T) { + notFound := &net.DNSError{Err: "no such host", Name: "invalid.example.com", IsNotFound: true} + if !rhttp.IsDNSNotFound(notFound) { + t.Error("expected IsDNSNotFound to be true for NXDOMAIN") + } + + transient := &net.DNSError{Err: "server misbehaving", IsTemporary: true} + if rhttp.IsDNSNotFound(transient) { + t.Error("expected IsDNSNotFound to be false for a transient DNS failure") + } + + if rhttp.IsDNSNotFound(nil) { + t.Error("expected IsDNSNotFound to be false for nil") + } +} + func TestIsTLS(t *testing.T) { if !rhttp.IsTLS(x509.UnknownAuthorityError{}) { t.Error("expected IsTLS to be true for TLS error") @@ -249,8 +327,10 @@ func TestClassify_AllKindsAreReachable(t *testing.T) { rhttp.ErrKindTimeout: context.DeadlineExceeded, rhttp.ErrKindCanceled: context.Canceled, rhttp.ErrKindConnection: syscall.ECONNREFUSED, - rhttp.ErrKindDNS: &net.DNSError{Err: "no such host"}, + rhttp.ErrKindDNS: &net.DNSError{Err: "server misbehaving"}, rhttp.ErrKindTLS: x509.UnknownAuthorityError{}, + + rhttp.ErrKindDNSNotFound: &net.DNSError{Err: "no such host", IsNotFound: true}, } for kind, err := range producers { diff --git a/retry_test.go b/retry_test.go index 3003f0c..5407d62 100644 --- a/retry_test.go +++ b/retry_test.go @@ -395,7 +395,8 @@ func TestRetry_RespectsErrorClassification(t *testing.T) { {"tls_not_retryable", tlsErr, 1}, {"canceled_not_retryable", context.Canceled, 1}, {"connection_retryable", syscall.ECONNREFUSED, 3}, - {"dns_retryable", &net.DNSError{Err: "no such host"}, 3}, + {"dns_transient_retryable", &net.DNSError{Err: "server misbehaving", IsTemporary: true}, 3}, + {"dns_notfound_not_retryable", &net.DNSError{Err: "no such host", IsNotFound: true}, 1}, {"timeout_retryable", context.DeadlineExceeded, 3}, } diff --git a/timeout.go b/timeout.go index 6304544..a71e5a1 100644 --- a/timeout.go +++ b/timeout.go @@ -38,7 +38,7 @@ func (t timeoutRoundTripper) RoundTrip(req *http.Request) (*http.Response, error ctx, cancel := context.WithTimeout(ctx, t.timeout) - req = req.Clone(ctx) + req = req.WithContext(ctx) resp, err := t.next.RoundTrip(req) if err != nil { cancel()