rework middleware and add frontend proxy #331
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Skipped Deployments
|
📝 WalkthroughWalkthroughThe pull request introduces a comprehensive refactoring of HTTP middleware components across multiple files. The changes standardize middleware implementation by introducing a consistent pattern of creating middleware structs with constructor functions and methods. Key modifications include centralizing configuration within middleware structs, updating method signatures to support more flexible middleware chaining, and adding new middleware components like frontend proxy and user agent context handling. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Middleware
participant Handler
Client->>Middleware: HTTP Request
Middleware->>Middleware: Apply CORS
Middleware->>Middleware: Log Request
Middleware->>Middleware: Frontend Proxy Check
Middleware->>Middleware: User Agent Context
Middleware->>Middleware: Session Authentication
Middleware->>Middleware: Rate Limiting
Middleware->>Handler: Forward Request
Handler-->>Middleware: Response
Middleware-->>Client: Return Response
Possibly Related PRs
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (12)
app/transports/http/middleware/chaos/chaos.go (1)
Line range hint
26-48: Use a logger instead offmt.Printlnfor chaos logs.Lines 33 and 41 utilize
fmt.Printlnfor debug output. Replacing them with a structured logger will align with best practices, providing consistent logs and supporting log levels.- fmt.Println("[DEV_CHAOS_SLOW_MODE] waiting", wait) + logger.Debug("dev chaos slow mode waiting", zap.Duration("wait", wait)) - fmt.Println("[DEV_CHAOS_FAIL_RATE] crashing") + logger.Warn("dev chaos fail rate triggered")app/transports/http/middleware/useragent/ua.go (1)
11-16: Include optional configuration fields in theMiddlewarestruct.Currently, the struct is empty. If future expansions (e.g., user agent filtering) are planned, consider adding relevant fields or placeholders.
app/transports/http/middleware/frontend/frontend.go (2)
15-17: Consider documenting the purpose ofProvider.Adding doc comments explaining that
Providermanages frontend proxy logic will help new contributors understand this middleware’s role.
42-58: Refactor conditionals for clarity.Within
WithFrontendProxy(), consider early returns for/apivs. non-API traffic to simplify the code structure. Current logic works, but short-circuiting can improve readability.app/transports/http/middleware/origin/cors.go (3)
12-14: Include configuration fields for multiple origins.Currently,
Middlewareonly stores the entire config. If multiple origins are needed or more advanced CORS settings, consider dedicated fields for clarity and future expansion.
16-18: Initialize default CORS settings or checks inNew.If you plan to configure multiple or dynamic origins, you might want to add default CORS logic here, ensuring that
WithCORSstays minimal and focused.
Line range hint
20-72: Use a full logger instead of potential no-op statements.If you decide to log CORS checks, consider introducing logging in this function. Currently, no logs are emitted. This could be useful for debugging or auditing.
app/transports/http/router.go (1)
32-38: Consider grouping multiple middleware parameters into a single struct.
Adding multiple middleware parameters to the function signature can lead to a large, unwieldy parameter list. Consider grouping them into a single struct or using constructor injection where appropriate, which can simplify dependency management and improve clarity.app/transports/http/middleware/reqlog/requestlog.go (1)
50-50: Add user or request identifiers to log context if available.
When logging requests, adding identifiers (e.g., user ID, correlation ID) can make debugging even more efficient, especially if your system has multi-step transactions or multiple services.app/transports/http/middleware/limiter/limiter.go (2)
16-20: Constants are well-defined, but consider dynamic configurability.
TheMaxRequestSizeBytesis set to 10 MB. If your system’s needs change, or if certain routes need different sizes, consider exposing this as a configurable parameter from yourcfg.
45-81: Rate-limiting logic is properly modularized.
Switching to a function returninghttp.Handlerfosters middleware chaining. You might consider extracting logic for calculating request cost from theTODOcomment, to a function or strategy pattern for clarity.docker/all/Dockerfile (1)
81-81: Ensure environment variables are well-documented.The addition of
PROXY_FRONTEND_ADDRESSis crucial for configuring the reverse proxy. Document it in the project’s configuration references or README so other developers can discover and adjust it according to their environment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
app/transports/http/middleware/chaos/chaos.go(1 hunks)app/transports/http/middleware/frontend/frontend.go(1 hunks)app/transports/http/middleware/limiter/limiter.go(3 hunks)app/transports/http/middleware/middleware.go(1 hunks)app/transports/http/middleware/origin/cors.go(1 hunks)app/transports/http/middleware/reqlog/requestlog.go(3 hunks)app/transports/http/middleware/session_cookie/cookie.go(1 hunks)app/transports/http/middleware/useragent/ua.go(1 hunks)app/transports/http/router.go(2 hunks)docker/all/Dockerfile(1 hunks)internal/config/config.go(1 hunks)
🔇 Additional comments (18)
app/transports/http/middleware/chaos/chaos.go (2)
12-16: Consider concurrency safety when storing chaos configuration.
Storing chaos configuration at the struct level is generally fine, but be mindful of potential concurrent usage. If future updates to these fields are planned during runtime, ensure proper synchronization.
18-24: Validate the logic for enabling chaos behavior.
Line 20's condition cfg.DevChaosFailRate > 0 || cfg.DevChaosSlowMode == 0 means that chaos is considered “enabled” if there is any fail rate or if the slow mode is zero. Make sure that this logic accurately reflects the desired behavior.
app/transports/http/middleware/middleware.go (2)
6-12: Organize imported middleware packages effectively.
The newly added imports (chaos, frontend, origin, reqlog, useragent) align well with the modular approach. Consider verifying that no unintended import cycles are introduced.
17-23: Ensure correct dependency injection ordering.
Registering middleware providers in this list is straightforward, but be aware of startup order if any middleware has cross-dependencies. Validate that each New function is sufficiently independent or arranged properly.
app/transports/http/middleware/useragent/ua.go (1)
19-33: Avoid overwriting request context if it includes a prior user agent.
Ensure that any existing user agent data in the request context is either replaced intentionally or combined if needed. This logic is fine as is, but some applications might prefer merging context values.
app/transports/http/middleware/frontend/frontend.go (2)
19-40: Validate HTTP proxy edge cases.
When cfg.FrontendProxy is non-nil, this logic properly builds a reverse proxy. Confirm that error handling and timeouts are configured if the proxied service is unreachable or slow.
60-64: Modularize the build step if more advanced logic is needed.
The current Build() function simply provides the New constructor. If advanced initialization or custom error handling is required later, break out logic into smaller functions for maintainability.
app/transports/http/router.go (2)
10-10: New dependency import looks consistent.
The import path for the chaos middleware is aligned with your project's structure, which simplifies future refactoring and keeps dependencies organized.
42-49: Review middleware chaining order for potential side effects.
Review the sequence in which middleware is applied (e.g., logging vs. chaos vs. rate limiting). For example, you might want logging to happen before chaos injection for better visibility on artificially induced failures. Ensure this order aligns with your observability and security needs.
app/transports/http/middleware/reqlog/requestlog.go (3)
13-15: Struct-based design promotes better encapsulation.
Defining a Middleware struct that holds the logger is a positive step, ensuring that configuration is kept close to the logic that uses it. This design makes the middleware easier to extend and maintain.
17-21: Constructor usage is straightforward and clear.
The New constructor clarifies how to acquire a logging-enabled middleware. This is a good practice for code readability and testability.
Line range hint 37-81: Thorough error handling ensures resilience.
The updated WithLogger method captures panics and logs them, then returns an HTTP 500 response. This robust error handling improves reliability. Consider verifying through integration tests that those logs are emitted with the expected level and format.
app/transports/http/middleware/limiter/limiter.go (3)
24-27: Enhanced state in the Middleware struct is clear and self-contained.
Including sizeLimit as part of the struct consolidates configuration. Verify that other areas referencing request size limits (if any) adapt to this single source of truth.
38-41: Constructor sets sensible defaults.
Your constructor picks up MaxRequestSizeBytes for sizeLimit. This centralizes logic, making it simpler to change defaults later.
102-105: Request size limiting is integrated consistently.
Wrapping the request body via http.MaxBytesReader is straightforward and effective. Remember to confirm error handling in controllers for incomplete reads if large requests are truncated.
internal/config/config.go (1)
16-19: New FrontendProxy field extends configuration flexibility.
The addition of FrontendProxy allows routing requests to a separate frontend service. Ensure that any references to this field handle potential nil values gracefully if the environment variable is unset.
app/transports/http/middleware/session_cookie/cookie.go (1)
124-131: Refactor to leverage consistent middleware chaining.
Great use of a higher-order function for consistent chaining. This aligns with the approach in other middleware components, resulting in a more flexible and uniform API. Note that the newly introduced closure correctly captures the j instance and properly injects the session context.
docker/all/Dockerfile (1)
79-79: Clarify the comment for RUN_FRONTEND usage.
Changing the comment to specify that the API process will run the Next.js server and proxy requests provides clarity on the relationship between the backend and frontend. This helps developers understand the underlying infrastructure better.
No description provided.