Conversation
…ntext-aware constructors & graceful shutdown Introduce an optional management HTTP server (Fiber v3) with endpoints: GET /health GET /stats GET /config POST /evict POST /trigger-expiration POST /clear Key additions and changes: - New option: WithManagementHTTP(...) to enable server - New file: management_http.go (server lifecycle, auth hook, graceful shutdown) - New methods: ManagementHTTPAddress(), TriggerExpiration(), EvictionInterval(), ExpirationInterval(), EvictionAlgorithm() - Added sentinel.ErrMgmtHTTPShutdownTimeout - README updated with usage and endpoint docs - Added integration test: management_http_test.go - Added Fiber dependency and related indirect modules; cspell dictionary updated - Example rename: observability file renamed for clarity Middleware & API adjustments: - Service.Stop now: Stop(ctx context.Context) error - Adapted logging, metrics, tracing, and stats middlewares to propagate context - HyperCache.New now requires a context for initializing (used for optional management server start) - NewInMemoryWithDefaults updated accordingly BREAKING CHANGE: func NewT signature changed to: New(ctx context.Context, bm *BackendManager, config *Config[T]) (*HyperCache[T], error) Service interface Stop method changed to: Stop(ctx context.Context) error All callers must pass a context and handle the error from Stop Any custom middleware or wrappers must update their Stop implementations Test: Added management HTTP endpoint coverage (basic liveness & JSON contract) Ref: Introduces graceful shutdown path for management server with timeout handling
|
Running Code Quality on PRs by uploading data to Trunk will soon be removed. You can still run checks on your PRs using trunk-action - see the migration guide for more information. |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a comprehensive management HTTP API feature alongside breaking changes to support context-aware operations and graceful shutdown. The management server provides runtime inspection and control endpoints for cache instances using Fiber v3.
- Adds optional management HTTP server with health, stats, config, and control endpoints
- Updates service interface to require context for Stop operations and return errors
- Integrates graceful shutdown handling for the management server with timeout control
Reviewed Changes
Copilot reviewed 19 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| management_http.go | New management HTTP server implementation with Fiber v3 and graceful shutdown |
| service.go | Breaking change: Stop method now requires context and returns error |
| hypercache.go | Adds management server integration, new accessor methods, and context-aware constructor |
| pkg/middleware/*.go | Updates all middleware to propagate context through Stop calls |
| tests/*.go | Updates test calls to use new context-aware API signatures |
| config.go | Adds WithManagementHTTP option for enabling the server |
| go.mod | Adds Fiber v3 dependency and related modules |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| })) | ||
| // clear cache | ||
| s.app.Post("/clear", useAuth(func(fiberCtx fiber.Ctx) error { | ||
| clearErr := hc.Clear(ctx) |
There was a problem hiding this comment.
The context passed to Clear() is the mount context from server startup, not the request context. This could cause issues if the mount context is canceled while a request is being processed. Use the request context instead by extracting it from fiberCtx.
| clearErr := hc.Clear(ctx) | |
| clearErr := hc.Clear(fiberCtx.Context()) |
| return err | ||
| } | ||
|
|
||
| cancel() |
There was a problem hiding this comment.
The context timeout is created but if the shutdown succeeds, cancel() is only called after the successful path. If an error occurs, cancel() is called but the timeout context might still be leaked in some error paths. Move cancel() to a defer statement to ensure it's always called.
| cancel() | |
| defer cancel() | |
| err := hyperCache.mgmtHTTP.Shutdown(ctx) | |
| if err != nil { | |
| // Handle error | |
| return err | |
| } |
tests/management_http_test.go
Outdated
| dec := json.NewDecoder(resp.Body) | ||
| err = dec.Decode(&statsBody) | ||
| assert.NoError(t, err) | ||
| assert.True(t, resp.StatusCode == http.StatusOK) |
There was a problem hiding this comment.
This assertion is redundant since line 48 already asserts the same condition with assert.Equal. Remove this duplicate assertion.
| assert.True(t, resp.StatusCode == http.StatusOK) |
| go func() { // serve in background (optional server errors are ignored intentionally) | ||
| err = s.app.Listener(ln) | ||
| if err != nil { // optional server; log hook could be added in future | ||
| _ = err |
There was a problem hiding this comment.
Silently ignoring server errors makes debugging difficult. Consider adding a comment explaining why the error is intentionally ignored or implement proper error logging.
| _ = err | |
| log.Printf("Management HTTP server stopped: %v", err) |
1 participant
Introduce an optional management HTTP server (Fiber v3) with endpoints:
GET /health
GET /stats
GET /config
POST /evict
POST /trigger-expiration
POST /clear
Key additions and changes:
Middleware & API adjustments:
BREAKING CHANGE:
func NewT signature changed to: New(ctx context.Context, bm *BackendManager, config *Config[T]) (*HyperCache[T], error) Service interface Stop method changed to: Stop(ctx context.Context) error All callers must pass a context and handle the error from Stop Any custom middleware or wrappers must update their Stop implementations
Test:
Added management HTTP endpoint coverage (basic liveness & JSON contract)
Ref:
Introduces graceful shutdown path for management server with timeout handling