🔥 feat: Native support for net/http and fasthttp handlers #3769
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds adapter helpers to convert net/http and fasthttp handler shapes into Fiber handlers, changes router/App/Group/Register/Router APIs to accept generic Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant App as App / Router
participant Collector as collectHandlers
participant Wrapper as Fiber Wrapper
participant FasthttpAdaptor as fasthttpadaptor
participant HTTP as net/http Handler
participant Ctx as fiber.Ctx
Dev->>App: Register route with http.HandlerFunc / http.Handler / fasthttp.RequestHandler
App->>Collector: collectHandlers(handler any, ...)
Collector-->>App: []Wrapper
Ctx->>Wrapper: invoke converted handler on request
alt converted net/http handler
Wrapper->>FasthttpAdaptor: adapt request
FasthttpAdaptor->>HTTP: ServeHTTP(ResponseWriter, *http.Request)
HTTP-->>FasthttpAdaptor: write headers/body
FasthttpAdaptor-->>Ctx: propagate response
else native Fiber handler
Wrapper->>Ctx: call Fiber handler directly
end
Ctx-->>App: complete response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)docs/**📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🪛 LanguageTooldocs/whats_new.md[uncategorized] ~304-~304: Do not mix variants of the same word (‘adapter’ and ‘adaptor’) within a single text. (EN_WORD_COHERENCY) [uncategorized] ~304-~304: Do not mix variants of the same word (‘adapter’ and ‘adaptor’) within a single text. (EN_WORD_COHERENCY) [uncategorized] ~304-~304: Do not mix variants of the same word (‘adapter’ and ‘adaptor’) within a single text. (EN_WORD_COHERENCY) [grammar] ~1629-~1629: There might be a mistake here. (QB_NEW_EN) [grammar] ~1630-~1630: There might be a mistake here. (QB_NEW_EN) [grammar] ~1631-~1631: There might be a mistake here. (QB_NEW_EN) [grammar] ~1636-~1636: There might be a mistake here. (QB_NEW_EN) [grammar] ~1637-~1637: There might be a mistake here. (QB_NEW_EN) Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
gaby
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3769 +/- ##
==========================================
+ Coverage 91.73% 92.02% +0.28%
==========================================
Files 113 115 +2
Lines 11966 12249 +283
==========================================
+ Hits 10977 11272 +295
+ Misses 727 712 -15
- Partials 262 265 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/gemini review |
There was a problem hiding this comment.
Pull Request Overview
This PR adds native support for net/http handlers in Fiber's router, allowing developers to pass http.Handler, http.HandlerFunc, or compatible function types directly to routing methods without manual adaptation.
- Converts handler parameter types from
Handlertoanyacross all router interfaces - Adds automatic conversion logic to detect and wrap
net/httphandlers using the existing adaptor - Updates documentation to reflect the new capability and show usage examples
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| adapter.go | New file containing handler conversion logic and utilities |
| adapter_test.go | Comprehensive tests for the new handler conversion functionality |
| app.go | Updated app-level routing methods to accept any type handlers with conversion |
| app_test.go | Added test demonstrating direct net/http handler registration |
| group.go | Updated group routing methods to support mixed handler types |
| register.go | Updated route chain registration methods for any type handlers |
| router.go | Updated Router interface to accept any type handlers |
| router_test.go | Added comprehensive tests for mixed Fiber/HTTP handler scenarios |
| docs/whats_new.md | Added documentation section about the new net/http handler support |
| docs/partials/routing/handler.md | Updated handler documentation with examples and new signatures |
| docs/middleware/adaptor.md | Updated adaptor docs to mention automatic conversion capability |
| docs/extra/faq.md | Updated FAQ to reference direct handler registration |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable feature by adding native support for net/http handlers in the Fiber router, which significantly improves interoperability. The implementation is clean, well-structured, and accompanied by thorough tests. I've identified a minor area for improvement to enhance code consistency.
gaby
changed the title
gaby
changed the title
|
Fixing lint and review comments in the next hour |
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting |
gaby
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
adapter.go (2)
10-15: Broaden GenericHandler and toFiberHandler to include http.Handler for symmetry.Small completeness tweak: add http.Handler to the constraint and handle it in toFiberHandler so all supported shapes go through one path.
// GenericHandler enumerates the handler shapes that can be bridged to Fiber. type GenericHandler interface { - Handler | - http.HandlerFunc | - func(http.ResponseWriter, *http.Request) + Handler | + http.Handler | + http.HandlerFunc | + func(http.ResponseWriter, *http.Request) } // toFiberHandler converts supported handler types to a Fiber handler. func toFiberHandler[T GenericHandler](handler T) Handler { switch h := any(handler).(type) { case Handler: return h + case http.Handler: + return wrapHTTPHandler(h) case http.HandlerFunc: return wrapHTTPHandler(h) case func(http.ResponseWriter, *http.Request): return wrapHTTPHandler(http.HandlerFunc(h)) default: return nil } }Also applies to: 17-29
45-60: Polish panic message and avoid ambiguous 'context' param name.
- Drop trailing newline in panic.
- Rename parameter to avoid shadowing common package names and improve clarity.
-// collectHandlers converts a slice of handler arguments to Fiber handlers. -// The context string is used to provide informative panic messages when an +// collectHandlers converts a slice of handler arguments to Fiber handlers. +// The where string is used to provide informative panic messages when an // unsupported handler type is encountered. -func collectHandlers(context string, args ...any) []Handler { +func collectHandlers(where string, args ...any) []Handler { handlers := make([]Handler, 0, len(args)) for _, arg := range args { handler, ok := convertHandler(arg) if !ok { - panic(fmt.Sprintf("%s: invalid handler %T\n", context, arg)) + panic(fmt.Sprintf("%s: invalid handler %T", where, arg)) } handlers = append(handlers, handler) } return handlers }Also applies to: 54-55
group.go (1)
85-90: Unify panic message (no trailing newline, use %T).Minor cleanup for consistency and clearer type reporting.
- handler, ok := convertHandler(arg) - if !ok { - panic(fmt.Sprintf("use: invalid handler %v\n", reflect.TypeOf(arg))) - } + handler, ok := convertHandler(arg) + if !ok { + panic(fmt.Sprintf("use: invalid handler %T", arg)) + } handlers = append(handlers, handler)app.go (1)
805-809: Unify panic message (no trailing newline, use %T).Mirror the group.Use() cleanup for consistency.
- handler, ok := convertHandler(arg) - if !ok { - panic(fmt.Sprintf("use: invalid handler %v\n", reflect.TypeOf(arg))) - } + handler, ok := convertHandler(arg) + if !ok { + panic(fmt.Sprintf("use: invalid handler %T", arg)) + } handlers = append(handlers, handler)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
adapter.go(1 hunks)adapter_test.go(1 hunks)app.go(2 hunks)group.go(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- adapter_test.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Format Go code using gofumpt (enforced viamake format)
Ensure code passes golangci-lint checks (enforced viamake lint)
Optimize struct field alignment using betteralign (enforced viamake betteralign)
Modernize Go code using gopls modernize (enforced viamake modernize)
Files:
adapter.goapp.gogroup.go
🧠 Learnings (1)
📚 Learning: 2024-11-10T23:44:13.704Z
Learnt from: gaby
PR: gofiber/fiber#3193
File: middleware/adaptor/adaptor.go:111-111
Timestamp: 2024-11-10T23:44:13.704Z
Learning: In the `middleware/adaptor/adaptor.go` file of the Fiber framework, when updating context handling, replacing `c.Context()` with `c.RequestCtx()` is appropriate to access the `fasthttp.RequestCtx`.
Applied to files:
adapter.go
🧬 Code graph analysis (3)
adapter.go (2)
app.go (1)
Handler(40-40)ctx_interface_gen.go (1)
Ctx(18-426)
app.go (3)
router.go (2)
App(366-385)Router(18-40)constants.go (9)
MethodGet(5-5)MethodHead(6-6)MethodPost(7-7)MethodPut(8-8)MethodDelete(10-10)MethodConnect(11-11)MethodOptions(12-12)MethodTrace(13-13)MethodPatch(9-9)group.go (1)
Group(14-21)
group.go (1)
router.go (1)
Router(18-40)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: repeated
- GitHub Check: Compare
🔇 Additional comments (7)
adapter.go (1)
31-43: LGTM: correct use of fasthttp adaptor and RequestCtx.The wrapper is concise and uses c.RequestCtx() per prior guidance. Based on learnings
group.go (3)
115-117: API now accepts 'any' for handlers — ensure misuse is covered by tests and docs.Broader types improve interop but allow invalid values to compile and panic at registration. Confirm docs call this out and tests cover bad types and typed-nil cases.
Also applies to: 121-123, 127-129, 133-135, 139-140, 145-146, 151-152, 156-158, 162-164, 178-181
167-170: LGTM: centralized normalization with collectHandlers in Add.Good consolidation; simplifies future handler-shape growth.
187-192: LGTM: Group middleware normalization via collectHandlers.Keeps group behavior consistent with app-level registration.
app.go (3)
831-833: Widened handler parameters to 'any' — confirm docs and tests for misuse and typed-nil.Same note as group.go: ensure guidance and tests exist for invalid types and nil handlers.
Also applies to: 837-839, 843-845, 849-851, 855-856, 861-862, 867-868, 873-874, 879-880, 891-893
883-886: LGTM: centralized normalization via collectHandlers in Add.Registration path is clean and consistent.
900-904: LGTM: Group uses collectHandlers for middleware normalization.Consistent with group.go.
…dlers. Add support for fasthttp.RequestHandler
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app_test.go (1)
284-284: Remove unnecessary loop variable copy.In Go 1.22+, loop variables are automatically scoped per iteration, making the
tt := ttcopy unnecessary. You can safely remove this line.Apply this diff:
for _, tt := range tests { - tt := tt t.Run(tt.name, func(t *testing.T) {adapter_test.go (1)
148-148: Remove unnecessary loop variable copy.In Go 1.22+, loop variables are automatically scoped per iteration, making the
tt := ttcopy unnecessary.Apply this diff:
for _, tt := range tests { - tt := tt t.Run(tt.name, func(t *testing.T) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
adapter.go(1 hunks)adapter_test.go(1 hunks)app_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Format Go code using gofumpt (enforced viamake format)
Ensure code passes golangci-lint checks (enforced viamake lint)
Optimize struct field alignment using betteralign (enforced viamake betteralign)
Modernize Go code using gopls modernize (enforced viamake modernize)
Files:
adapter.goadapter_test.goapp_test.go
🧠 Learnings (1)
📚 Learning: 2024-11-10T23:44:13.704Z
Learnt from: gaby
PR: gofiber/fiber#3193
File: middleware/adaptor/adaptor.go:111-111
Timestamp: 2024-11-10T23:44:13.704Z
Learning: In the `middleware/adaptor/adaptor.go` file of the Fiber framework, when updating context handling, replacing `c.Context()` with `c.RequestCtx()` is appropriate to access the `fasthttp.RequestCtx`.
Applied to files:
adapter.go
🧬 Code graph analysis (3)
adapter.go (2)
app.go (1)
Handler(40-40)ctx_interface_gen.go (1)
Ctx(18-426)
adapter_test.go (3)
app.go (2)
Handler(40-40)New(515-628)ctx_interface_gen.go (1)
Ctx(18-426)constants.go (3)
StatusAccepted(54-54)StatusTeapot(91-91)StatusNoContent(56-56)
app_test.go (1)
app.go (2)
App(68-110)New(515-628)
🪛 GitHub Actions: golangci-lint
adapter_test.go
[error] 129-129: fieldalignment: struct with 32 pointer bytes could be 24 (govet)
🪛 GitHub Check: lint
adapter_test.go
[failure] 148-148:
The copy of the 'for' variable "tt" can be deleted (Go 1.22+) (copyloopvar)
[failure] 129-129:
fieldalignment: struct with 32 pointer bytes could be 24 (govet)
app_test.go
[failure] 284-284:
The copy of the 'for' variable "tt" can be deleted (Go 1.22+) (copyloopvar)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: unit (1.25.x, windows-latest)
- GitHub Check: repeated
- GitHub Check: Compare
🔇 Additional comments (7)
adapter.go (4)
39-46: LGTM!The fasthttp.RequestHandler conversion is well-implemented. The nil check and the use of
c.RequestCtx()align with the framework's patterns for accessing the underlying fasthttp context.Based on learnings
52-64: LGTM!The HTTP-to-Fiber adapter is correctly implemented. The use of
fasthttpadaptor.NewFastHTTPHandlerandc.RequestCtx()follows the framework's established patterns for bridging net/http handlers.Based on learnings
66-81: LGTM!The handler collection logic is well-implemented with clear, contextual error messages that include both the handler index and type information, making debugging easier for developers.
83-91: LGTM!The nil handling and delegation pattern is clean and appropriate for this normalizer function.
app_test.go (1)
201-323: LGTM! Comprehensive test coverage for net/http handler registration.The test effectively validates that net/http handlers can be registered via all HTTP method routing functions. The special handling for Connect requests (setting scheme/host) and the HEAD method (no body expectation) demonstrates thorough testing.
adapter_test.go (2)
14-236: LGTM! Comprehensive test coverage for adapter functions.The tests thoroughly cover various conversion scenarios including nil handling, type conversions, mixed handlers, and panic cases. The use of
t.Cleanupfor resource management and parallel test execution are good practices.
129-145: Address fieldalignment linter issue.The pipeline is failing due to a fieldalignment issue on the test struct. While this appears to be a false positive (the struct only has 2 fields and reordering won't help), you can add a nolint directive to suppress the warning.
Apply this diff:
+ //nolint:govet // fieldalignment: false positive - struct has only 2 fields tests := []struct { name string handler anyAlternatively, verify if there's a way to satisfy the linter by restructuring the test data.
|
/gemini review |
gaby
changed the title
There was a problem hiding this comment.
Code Review
This pull request introduces a great new feature by adding native support for net/http handlers in Fiber. The implementation is solid, with the core conversion logic well-encapsulated in the new adapter.go file. The changes are consistently applied across the router, group, and registration interfaces, and the new functionality is well-tested, including important edge cases. The documentation updates are also thorough and clearly explain the new feature to users. I have a couple of suggestions to further improve the code and documentation.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app_test.go (2)
283-322: LGTM! Test execution is sound with correct HTTP semantics.The test correctly:
- Creates net/http handlers and registers them via Fiber's routing methods
- Handles HEAD requests (no body write) and CONNECT requests (URL scheme/host) appropriately
- Verifies status code, headers, and body content
Optional: Consider using
require.NoErrorfor faster test failure.On line 296, using
require.NoError(t, err)instead ofassert.NoError(t, err)would cause the test to fail immediately if the write fails, providing clearer error messages rather than continuing with a corrupted response.Apply this diff:
- _, err := w.Write([]byte("hello from net/http " + r.Method)) - assert.NoError(t, err) + _, err := w.Write([]byte("hello from net/http " + r.Method)) + require.NoError(t, err)
218-224: Optional: MakeexpectBodyexplicit for HEAD case.While the current code is correct (omitted
expectBodydefaults tofalse), explicitly settingexpectBody: falsewould improve clarity and make the test case's intent more obvious.Apply this diff:
{ name: "Head", register: func(app *App, path string, handler any) { app.Head(path, handler) }, - method: http.MethodHead, + method: http.MethodHead, + expectBody: false, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
adapter_test.go(1 hunks)app_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- adapter_test.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Format Go code using gofumpt (enforced viamake format)
Ensure code passes golangci-lint checks (enforced viamake lint)
Optimize struct field alignment using betteralign (enforced viamake betteralign)
Modernize Go code using gopls modernize (enforced viamake modernize)
Files:
app_test.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Compare
- GitHub Check: unit (1.25.x, macos-13)
- GitHub Check: unit (1.25.x, windows-latest)
- GitHub Check: repeated
🔇 Additional comments (1)
app_test.go (1)
201-281: LGTM! Comprehensive test coverage for all HTTP methods.The table-driven test structure is well-designed and covers all standard HTTP methods (GET, HEAD, POST, PUT, DELETE, CONNECT, OPTIONS, TRACE, PATCH). The
expectBodyflag appropriately distinguishes HEAD requests from others, and the register functions correctly adapt each method for testing.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a significant feature: native support for net/http and fasthttp handlers in Fiber's router. The changes are extensive, modifying router interfaces across app, group, and register to accept any type for handlers, and introducing a new adapter.go file with the core conversion logic. The implementation is robust, with comprehensive test coverage for various handler types and edge cases, including typed and untyped nils. The documentation has also been thoroughly updated to reflect these new capabilities and their performance implications. My review includes a suggestion to refactor the handler collection logic for improved clarity and maintainability by centralizing nil checks.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
adapter.go (1)
69-92: Consider simplifying the nil handling.The explicit
case nilin the switch statement is redundant sincetoFiberHandleralready handles nil by returning(nil, false). While the current implementation is correct and explicit, it could be simplified for consistency.You can simplify by removing the switch statement entirely:
func collectHandlers(context string, args ...any) []Handler { handlers := make([]Handler, 0, len(args)) for i, arg := range args { - var ( - handler Handler - ok bool - ) - - switch h := arg.(type) { - case nil: - handler, ok = nil, false - default: - handler, ok = toFiberHandler(h) - } + handler, ok := toFiberHandler(arg) if !ok { panic(fmt.Sprintf("%s: invalid handler #%d (%T)\n", context, i, arg)) } handlers = append(handlers, handler) } return handlers }This centralizes all type checking in
toFiberHandler, making the code more maintainable. However, the current explicit approach is also clear, so this is optional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/README.md(1 hunks)adapter.go(1 hunks)adapter_test.go(1 hunks)app.go(2 hunks)docs/extra/faq.md(1 hunks)docs/partials/routing/handler.md(1 hunks)docs/whats_new.md(2 hunks)group.go(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/extra/faq.md
- docs/whats_new.md
- docs/partials/routing/handler.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Format Go code using gofumpt (enforced viamake format)
Ensure code passes golangci-lint checks (enforced viamake lint)
Optimize struct field alignment using betteralign (enforced viamake betteralign)
Modernize Go code using gopls modernize (enforced viamake modernize)
Files:
adapter.gogroup.goadapter_test.goapp.go
🧠 Learnings (1)
📚 Learning: 2024-11-10T23:44:13.704Z
Learnt from: gaby
PR: gofiber/fiber#3193
File: middleware/adaptor/adaptor.go:111-111
Timestamp: 2024-11-10T23:44:13.704Z
Learning: In the `middleware/adaptor/adaptor.go` file of the Fiber framework, when updating context handling, replacing `c.Context()` with `c.RequestCtx()` is appropriate to access the `fasthttp.RequestCtx`.
Applied to files:
adapter.go.github/README.md
🧬 Code graph analysis (4)
adapter.go (2)
app.go (1)
Handler(40-40)ctx_interface_gen.go (1)
Ctx(18-426)
group.go (1)
router.go (1)
Router(18-40)
adapter_test.go (2)
app.go (2)
Handler(40-40)New(515-628)ctx_interface_gen.go (1)
Ctx(18-426)
app.go (2)
router.go (2)
App(366-385)Router(18-40)group.go (1)
Group(14-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Compare
- GitHub Check: repeated
- GitHub Check: unit (1.25.x, windows-latest)
🔇 Additional comments (10)
.github/README.md (1)
128-128: LGTM! Clear and accurate description of the new capability.The updated wording accurately reflects the dual approach: automatic adaptation for common handler shapes at registration time, while preserving the adaptor middleware for more complex scenarios.
adapter_test.go (1)
14-244: LGTM! Comprehensive test coverage for adapter functionality.The test suite thoroughly covers:
- Nil handling (typed and untyped)
- Fiber handler preservation without wrapping
- HTTP handler shape conversions (http.HandlerFunc, http.Handler, raw function)
- Fasthttp handler support
- Mixed handler collections
- Invalid type panics with clear error messages
- Resource management via t.Cleanup
All tests properly verify the conversion behavior and maintain parallelism for performance.
adapter.go (2)
13-50: LGTM! Comprehensive handler conversion with proper nil handling.The function correctly handles all common handler shapes and properly rejects nil values (both typed and untyped) by returning
(nil, false), which allowscollectHandlersto panic with a clear error message.The use of
c.RequestCtx()at line 44 is correct for accessing the underlying fasthttp context. Based on learnings.
52-64: LGTM! Proper net/http to Fiber adapter using fasthttpadaptor.The implementation correctly uses
fasthttpadaptor.NewFastHTTPHandlerto bridge net/http handlers. The nil guard prevents issues, and the closure pattern properly invokes the adapted handler with the fasthttp context. Based on learnings.group.go (3)
85-89: LGTM! Consistent handler conversion in Use() method.The change properly uses
toFiberHandlerto normalize handler inputs, maintaining consistency with the app-levelUse()method. The panic message provides clear context for debugging.
167-169: LGTM! Proper batch handler conversion.The
Add()method correctly usescollectHandlersto normalize all handlers (primary + variadic) before registration. The "group" context string provides clear error context.
187-191: LGTM! Consistent handler conversion in Group() method.The conditional conversion properly handles the optional middleware case, only calling
collectHandlerswhen handlers are provided.app.go (3)
805-809: LGTM! Simplified and consistent Use() implementation.The method correctly uses
toFiberHandlerin the default case, which handles all supported types includingHandler. The redundant type-specific cases mentioned in past reviews have been removed.
883-885: LGTM! Proper handler conversion in Add() method.The implementation correctly uses
collectHandlersto normalize all handlers before registration, maintaining consistency with the group-levelAdd()method.
899-903: LGTM! Consistent Group() method implementation.The conditional handler conversion mirrors the pattern in
group.go, properly normalizing middleware only when provided.
Summary
Adds native support for net/http and fasthttp handlers in Fiber's router, allowing developers to pass http.Handler, http.HandlerFunc, fasthttp.RequestHandler or compatible function types directly to routing methods without manual adaptation:
Fixes #2202