From ed9feffa42b6432bd8ae77e046045dc442120b59 Mon Sep 17 00:00:00 2001 From: Festus Date: Sun, 8 Feb 2026 17:23:55 +0100 Subject: [PATCH] fix: address HTTP client feedback from issue #6 - Replace panic with proper error in Call() method - Add jitter to retry logic to prevent thundering herd - Add status-code-based retry with WithRetryOnStatus() - Update examples to use microhttp import alias - Add DESIGN.md to clarify project philosophy Fixes #6 --- DESIGN.md | 122 ++++++++++++++++++++++++++++++++++ README.md | 15 ++++- adapters/http/client.go | 15 ++++- examples/network/http/main.go | 18 +++-- internal/retry/retry.go | 12 +++- network/options.go | 23 ++++++- network/types.go | 9 ++- 7 files changed, 200 insertions(+), 14 deletions(-) create mode 100644 DESIGN.md diff --git a/DESIGN.md b/DESIGN.md new file mode 100644 index 0000000..ca9f182 --- /dev/null +++ b/DESIGN.md @@ -0,0 +1,122 @@ +# Design Philosophy + +## Core Principles + +### 1. Minimal Convenience Layer +microkit is **not a framework**. It's a toolkit that provides thin abstractions over common infrastructure patterns while staying close to the Go standard library. + +**What this means:** +- Wraps stdlib types, doesn't replace them +- Minimal API surface +- Easy to understand by reading the source +- No magic, no code generation + +### 2. Composable by Default +Every component should work independently and compose naturally with existing Go ecosystem tools. + +**What this means:** +- No global state +- Explicit dependencies +- Standard interfaces (context.Context, io.Reader, etc.) +- Easy to wrap or extend + +### 3. Opinionated Defaults, Flexible Overrides +Provide sensible defaults for common use cases, but allow users to override when needed. + +**What this means:** +- Retry with exponential backoff + jitter by default +- Graceful shutdown patterns built-in +- But users can bring their own implementations + +## What microkit IS + +✅ Clean abstractions for messaging (Kafka, RabbitMQ) +✅ Network client wrappers with retry/timeout +✅ Common patterns (retry, circuit breaker, graceful shutdown) +✅ Minimal boilerplate for microservice infrastructure + +## What microkit is NOT + +❌ A web framework (use stdlib, chi, echo, etc.) +❌ An observability platform (use OpenTelemetry) +❌ A service mesh (use Istio, Linkerd, etc.) +❌ A complete microservice framework (use go-kit, go-micro, etc.) + +## Extension Points + +While microkit stays minimal, it provides extension points for common needs: + +### Middleware/Hooks (Planned) +```go +// Optional middleware interface for HTTP clients +type Middleware func(next RoundTripper) RoundTripper + +client := microhttp.NewClient(10*time.Second, + microhttp.WithMiddleware(tracingMiddleware), + microhttp.WithMiddleware(metricsMiddleware), +) +``` + +### Custom Retry Logic +```go +// Users can implement custom retry strategies +type RetryStrategy interface { + ShouldRetry(attempt int, err error, resp *Response) bool + NextDelay(attempt int) time.Duration +} +``` + +## Design Decisions + +### Why panic → error in Call()? +**Problem:** HTTP client implements network.Client interface which has a Call() method for gRPC. HTTP client doesn't support gRPC, so it panicked. + +**Why panic was wrong:** Libraries should never panic for expected conditions. Panic means "programmer error, crash the process"—too harsh for a library. + +**Solution:** Return `network.ErrUnsupportedOperation` instead. + +### Why jitter in retry? +**Problem:** Without jitter, all clients retry at the same intervals, causing thundering herd. + +**Solution:** Add randomization (50-100% of delay) to spread out retry attempts. + +### Why status-code-based retry? +**Problem:** Some failures (429 rate limit, 503 service unavailable) should be retried even though the HTTP request succeeded. + +**Solution:** Add `WithRetryOnStatus()` option to retry specific status codes. + +### Why import alias recommendation? +**Problem:** Package named `http` collides with `net/http`, forcing users to alias one of them. + +**Solution:** Recommend aliasing microkit's package as `microhttp` in all docs/examples. + +## Future Considerations + +### Observability Hooks (Under Discussion) +- Optional hooks for tracing (OpenTelemetry compatible) +- Optional hooks for metrics (Prometheus compatible) +- **Key:** Must be opt-in, not required + +### Structured Logging (Under Discussion) +- Should microkit log internally? +- If yes, use slog (stdlib) and allow injection +- If no, return errors and let users log + +### Testing Utilities (Planned) +- Mock implementations of interfaces +- Test helpers for common scenarios +- In-memory message brokers for testing + +## Contributing + +When proposing new features, ask: +1. Is this a common pattern across microservices? +2. Does it reduce boilerplate significantly? +3. Can it be implemented as a thin wrapper over stdlib/existing tools? +4. Does it stay composable with the ecosystem? + +If yes to all four, it's probably a good fit for microkit. + +## Questions? + +Open an issue or discussion to talk about design decisions, scope, or future direction. diff --git a/README.md b/README.md index 44af26d..9129de5 100644 --- a/README.md +++ b/README.md @@ -45,12 +45,12 @@ import ( "fmt" "time" - "github.com/festus/microkit/adapters/http" + microhttp "github.com/festus/microkit/adapters/http" "github.com/festus/microkit/network" ) func main() { - client := http.NewClient(10 * time.Second) + client := microhttp.NewClient(10 * time.Second) defer client.Close() resp, err := client.Get(context.Background(), "https://api.example.com", @@ -145,7 +145,7 @@ producer.Publish(ctx, []byte("key"), []byte(`{"id":"123"}`)) ### HTTP Client with Retry ```go -client := http.NewClient(10 * time.Second) +client := microhttp.NewClient(10 * time.Second) defer client.Close() resp, _ := client.Get(ctx, "https://api.example.com/users", @@ -154,6 +154,15 @@ resp, _ := client.Get(ctx, "https://api.example.com/users", ) ``` +### HTTP Client with Status Code Retry + +```go +// Retry on 429 (rate limit) and 503 (service unavailable) +resp, _ := client.Get(ctx, "https://api.example.com/users", + network.WithRetryOnStatus(3, 100*time.Millisecond, 2*time.Second, 2.0, 429, 503), +) +``` + ### gRPC Client with Retry ```go diff --git a/adapters/http/client.go b/adapters/http/client.go index ed54878..3e53b8a 100644 --- a/adapters/http/client.go +++ b/adapters/http/client.go @@ -44,7 +44,7 @@ func (c *Client) Delete(ctx context.Context, url string, opts ...network.Option) } func (c *Client) Call(ctx context.Context, method string, req interface{}, resp interface{}, opts ...network.Option) error { - panic("gRPC not supported by HTTP client") + return network.ErrUnsupportedOperation } func (c *Client) do(ctx context.Context, method, url string, body []byte, opts ...network.Option) (*network.Response, error) { @@ -58,7 +58,18 @@ func (c *Client) do(ctx context.Context, method, url string, body []byte, opts . err := retry.Execute(ctx, *config.RetryConfig, func() error { var err error resp, err = c.doRequest(ctx, method, url, body, config) - return err + if err != nil { + return err + } + // Retry on specific status codes if configured + if len(config.RetryStatusCodes) > 0 { + for _, code := range config.RetryStatusCodes { + if resp.StatusCode == code { + return retry.ErrRetryableStatus + } + } + } + return nil }) return resp, err } diff --git a/examples/network/http/main.go b/examples/network/http/main.go index bb2e8fc..62b6cb0 100644 --- a/examples/network/http/main.go +++ b/examples/network/http/main.go @@ -6,17 +6,18 @@ import ( "log" "time" - "github.com/festus/microkit/adapters/http" + microhttp "github.com/festus/microkit/adapters/http" "github.com/festus/microkit/network" ) func main() { - client := http.NewClient(10 * time.Second) + client := microhttp.NewClient(10 * time.Second) defer client.Close() ctx := context.Background() - // HTTP GET with retry + // Example 1: HTTP GET with retry on errors + fmt.Println("Example 1: Basic retry on errors") resp, err := client.Get(ctx, "https://httpbin.org/get", network.WithHeader("User-Agent", "microkit/1.0"), network.WithRetry(3, 100*time.Millisecond, 2*time.Second, 2.0), @@ -24,6 +25,15 @@ func main() { if err != nil { log.Fatal(err) } + fmt.Printf("Response: %d\n\n", resp.StatusCode) - fmt.Printf("Response: %d\n", resp.StatusCode) + // Example 2: Retry on specific status codes (429, 503) + fmt.Println("Example 2: Retry on status codes 429 and 503") + resp2, err := client.Get(ctx, "https://httpbin.org/status/200", + network.WithRetryOnStatus(3, 100*time.Millisecond, 2*time.Second, 2.0, 429, 503), + ) + if err != nil { + log.Fatal(err) + } + fmt.Printf("Response: %d\n", resp2.StatusCode) } \ No newline at end of file diff --git a/internal/retry/retry.go b/internal/retry/retry.go index 4644e9d..5827ed1 100644 --- a/internal/retry/retry.go +++ b/internal/retry/retry.go @@ -2,14 +2,19 @@ package retry import ( "context" + "errors" + "math/rand" "time" ) +var ErrRetryableStatus = errors.New("retryable status code") + type Config struct { MaxAttempts int InitialDelay time.Duration MaxDelay time.Duration Multiplier float64 + Jitter bool } func Execute(ctx context.Context, config Config, fn func() error) error { @@ -23,10 +28,15 @@ func Execute(ctx context.Context, config Config, fn func() error) error { } if attempt < config.MaxAttempts-1 { + actualDelay := delay + if config.Jitter { + actualDelay = time.Duration(float64(delay) * (0.5 + rand.Float64()*0.5)) + } + select { case <-ctx.Done(): return ctx.Err() - case <-time.After(delay): + case <-time.After(actualDelay): } delay = time.Duration(float64(delay) * config.Multiplier) diff --git a/network/options.go b/network/options.go index f92893c..f3e4c59 100644 --- a/network/options.go +++ b/network/options.go @@ -11,9 +11,10 @@ type Option func(*Config) // Config holds configuration for network requests. type Config struct { - Headers map[string]string - Timeout time.Duration - RetryConfig *retry.Config + Headers map[string]string + Timeout time.Duration + RetryConfig *retry.Config + RetryStatusCodes []int } // WithHeader adds a header to the request. @@ -34,6 +35,7 @@ func WithTimeout(timeout time.Duration) Option { } // WithRetry enables retry logic for network calls. +// Set jitter to true to add randomization and prevent thundering herd. func WithRetry(maxAttempts int, initialDelay, maxDelay time.Duration, multiplier float64) Option { return func(c *Config) { c.RetryConfig = &retry.Config{ @@ -41,6 +43,21 @@ func WithRetry(maxAttempts int, initialDelay, maxDelay time.Duration, multiplier InitialDelay: initialDelay, MaxDelay: maxDelay, Multiplier: multiplier, + Jitter: true, } } +} + +// WithRetryOnStatus enables retry based on HTTP status codes. +func WithRetryOnStatus(maxAttempts int, initialDelay, maxDelay time.Duration, multiplier float64, statusCodes ...int) Option { + return func(c *Config) { + c.RetryConfig = &retry.Config{ + MaxAttempts: maxAttempts, + InitialDelay: initialDelay, + MaxDelay: maxDelay, + Multiplier: multiplier, + Jitter: true, + } + c.RetryStatusCodes = statusCodes + } } \ No newline at end of file diff --git a/network/types.go b/network/types.go index e020abd..c9d519a 100644 --- a/network/types.go +++ b/network/types.go @@ -1,5 +1,7 @@ package network +import "errors" + // Response represents a network response. type Response struct { StatusCode int @@ -13,4 +15,9 @@ type Request struct { URL string Headers map[string]string Body []byte -} \ No newline at end of file +} + +// Common errors +var ( + ErrUnsupportedOperation = errors.New("operation not supported by this client") +) \ No newline at end of file