Update to v3#1391
Conversation
v3: Update Websocket to Fiber v3
chore: upgrade Fiber framework from v2 to v3
…h-RequestCtx Correct fasthttp context with c.RequestCtx()
# Conflicts: # jwt/go.mod # jwt/go.sum # jwt/utils.go # websocket/go.mod # websocket/go.sum
git checkout main -- . ':!.github' ':!*.md' find . -name go.mod -execdir sh -c 'echo "=== $(pwd) ==="; fiber migrate --to v3.0.0-beta.5 -f --hash="323d2c85c4a1e9444802c4253f5cb784a61290df"' \;
git checkout main -- . ':!.github' ':!*.md' find . -name go.mod -execdir sh -c 'echo "=== $(pwd) ==="; fiber migrate --to v3.0.0-beta.5 -f --hash="323d2c85c4a1e9444802c4253f5cb784a61290df"' \;
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
ReneWerner87
requested review from
efectn,
gaby and
sixcolors
and removed request for
a team
WalkthroughProject-wide migration from Fiber v2 to v3: updates imports, handler/context signatures (from *fiber.Ctx to fiber.Ctx), test harness (App.Test uses fiber.TestConfig), and logging/tracing integrations. JWT and PASETO middlewares remove ContextKey and add FromContext helpers. WebSocket, OTel, and various contrib middlewares adjust configs/callbacks. Adds a parallel Go test runner script. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Fiber as Fiber App
participant JWT as jwtware.Middleware
participant Handler as Protected Handler
Client->>Fiber: HTTP Request with Authorization
Fiber->>JWT: Invoke middleware (c fiber.Ctx)
JWT->>JWT: Extract token (header/query/param/cookie)
alt token valid
JWT->>Fiber: c.Locals(tokenKey, token)
note right of JWT: New helper<br/>FromContext(c) reads token
JWT->>Fiber: Next()
Fiber->>Handler: Call handler (c)
Handler->>Fiber: jwtware.FromContext(c)
Fiber-->>Client: 2xx/handler response
else invalid/missing
JWT-->>Client: Error via ErrorHandler
end
sequenceDiagram
autonumber
participant Client
participant Fiber as Fiber App
participant WS as websocket.Middleware
participant Upgrader as Upgrader
Client->>Fiber: HTTP GET /ws (Upgrade: websocket)
Fiber->>WS: Invoke (c fiber.Ctx)
WS->>WS: IsWebSocketUpgrade(c)
alt Upgrade true
WS->>Upgrader: Upgrade(c.RequestCtx())
Upgrader-->>WS: Success
WS-->>Client: 101 Switching Protocols
else
WS-->>Client: 426 Upgrade Required
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @ReneWerner87, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request represents a substantial effort to migrate the entire suite of Fiber contrib modules to be fully compatible with Fiber v3. The changes aim to modernize the codebase, align with the latest Fiber API conventions, and ensure that these middleware and utilities can seamlessly integrate with new Fiber applications. This comprehensive update enhances the overall stability, maintainability, and future-readiness of the contrib ecosystem.
Highlights
- Fiber v3 Upgrade: All
contribmodules have been updated to ensure full compatibility withgithub.com/gofiber/fiber/v3. This includes significant changes to middleware handler signatures, now passingfiber.Ctxby value instead of by pointer, aligning with Fiber v3's API. - Go Version Bump: The minimum Go version requirement across all
contribmodules has been raised toGo 1.25, ensuring compatibility with modern Go features and toolchains. - Improved Testing Infrastructure: A new
parallel-go-test.shscript has been introduced to facilitate parallel testing of Go modules, enhancing the efficiency of development and CI/CD workflows. Test assertions have also been migrated tostretchr/testify/assertfor consistency. - Dependency Modernization: All
go.modandgo.sumfiles have been thoroughly updated to reflect the latest compatible versions of both direct and indirect dependencies, contributing to a more secure and up-to-date project. - Context Handling Refinements: Specific modules like
fibernewrelicandpasetohave seen updates to their context handling mechanisms, including the introduction of new helper functions likeFromContextfor more idiomatic access to request-scoped data.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| //Propagate tracing context as headers in outbound response | ||
| tracingHeaders := make(propagation.HeaderCarrier) |
There was a problem hiding this comment.
[P1] Propagation span never attached to fiber context
The middleware now creates ctx with tracer.Start, but the previous call to c.SetUserContext(ctx) was removed and nothing replaces it. As a result cfg.Propagators.Inject(c, tracingHeaders) uses the original request context, so the generated span isn’t propagated to downstream handlers or response headers. This breaks distributed tracing because oteltrace.SpanFromContext(c) after the middleware returns nil. Use the context returned from tracer.Start when injecting headers and expose it on the request (via the v3 equivalent of SetUserContext).
Useful? React with 👍 / 👎.
| }, | ||
| }) | ||
|
|
||
| c.SetUserContext(newrelic.NewContext(c.UserContext(), txn)) | ||
| // TODO: SetUserContext was removed, please migrate manually: c.SetUserContext(newrelic.NewContext(c, txn)) |
There was a problem hiding this comment.
[P1] New Relic transaction never stored on request context
After creating the New Relic transaction the middleware used to call c.SetUserContext(newrelic.NewContext(...)) so downstream code could retrieve the transaction via FromContext. The migration removed that call without providing an alternative, leaving the request context unmodified and FromContext returning nil. The transaction should be attached to the request context using the v3 replacement for SetUserContext so other middleware/handlers can record attributes and errors on the same transaction.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Code Review
This pull request updates the repository to support Fiber v3. The changes are extensive, touching many modules to align with the new v3 API, including dependency updates and signature changes. A new parallel testing script has also been added. The migration seems mostly correct, but I've identified a few issues, primarily in documentation and a new script, that should be addressed to ensure clarity and correctness for users and contributors.
| | Property | Type | Description | Default | | ||
| |:---------------|:--------------------------------|:--------------------------------------------------------------------------------------------------------------------------------------------------------|:-----------------------------| | ||
| | Filter | `func(fiber.Ctx) bool` | Defines a function to skip middleware | `nil` | | ||
| | SuccessHandler | `func(fiber.Ctx) error` | SuccessHandler defines a function which is executed for a valid token. | `nil` | | ||
| | ErrorHandler | `func(fiber.Ctx, error) error` | ErrorHandler defines a function which is executed for an invalid token. | `401 Invalid or expired JWT` | | ||
| | SigningKey | `interface{}` | Signing key to validate token. Used as fallback if SigningKeys has length 0. | `nil` | | ||
| | SigningKeys | `map[string]interface{}` | Map of signing keys to validate token with kid field usage. | `nil` | | ||
| | Claims | `jwt.Claims` | Claims are extendable claims data defining token content. | `jwt.MapClaims{}` | | ||
| | TokenLookup | `string` | TokenLookup is a string in the form of `<source>:<name>` that is used | ` |
There was a problem hiding this comment.
The configuration table seems to be incomplete and malformed after the changes. Several important configuration options like AuthScheme, KeyFunc, and JWKSetURLs are missing, and the entry for TokenLookup is cut off. The previous, more detailed table should be restored and updated for v3.
| if [ "$RUN_GOMOD" -eq 1 ]; then | ||
| local mod_step_log="$TMPDIR/$safe.modlog" | ||
| : > "$mod_step_log" | ||
| ( cd "$module_dir" 2>/dev/null && go mod download >>"$mod_step_log" 2>&1 ) || true |
There was a problem hiding this comment.
The redirection of stderr from cd to /dev/null (2>/dev/null) hides potentially useful error messages if the directory cannot be entered (e.g., due to permissions or it being removed). While the && ensures the subsequent command doesn't run in the wrong directory, obscuring the cd error makes debugging harder. It's better to remove 2>/dev/null to allow cd to report errors. This applies to similar constructs on lines 237 and 248 as well.
| ( cd "$module_dir" 2>/dev/null && go mod download >>"$mod_step_log" 2>&1 ) || true | |
| ( cd "$module_dir" && go mod download >>"$mod_step_log" 2>&1 ) || true |
| }) | ||
|
|
||
| c.SetUserContext(newrelic.NewContext(c.UserContext(), txn)) | ||
| // TODO: SetUserContext was removed, please migrate manually: c.SetUserContext(newrelic.NewContext(c, txn)) |
There was a problem hiding this comment.
This TODO comment is misleading as it suggests using c.SetUserContext, which has been removed in Fiber v3. Since the context can no longer be replaced for downstream handlers, the transaction won't be available via newrelic.FromContext(c). This is a significant breaking change that needs to be addressed, perhaps by using c.Locals or another mechanism to propagate the transaction.
|
|
||
| // pass the span through userContext | ||
| c.SetUserContext(ctx) | ||
| // TODO: SetUserContext was removed, please migrate manually: c.SetUserContext(ctx) |
There was a problem hiding this comment.
This TODO comment is misleading as it suggests using c.SetUserContext, which has been removed in Fiber v3. This is a significant breaking change. A more helpful comment would guide the user on how to adapt. For example, it could clarify that oteltrace.SpanFromContext(c) should be used by downstream handlers to access the span, as the context is no longer replaced. This also applies to the TODO on line 164.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 30
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (46)
hcaptcha/hcaptcha_test.go (1)
35-39: Movedefer res.Body.Close()after the error check to prevent nil dereference
In hcaptcha/hcaptcha_test.go, update:-res, err := app.Test(req, fiber.TestConfig{Timeout: 0, FailOnTimeout: false}) -defer res.Body.Close() -if err != nil { - t.Fatal(err) -} +res, err := app.Test(req, fiber.TestConfig{Timeout: 0, FailOnTimeout: false}) +if err != nil { + t.Fatal(err) +} +defer res.Body.Close()fiberzerolog/README.md (2)
51-56: Example does not compile: missing os import and outdated handler signature.
- Add
osto imports (used at Line 59).- Switch handler to value
fiber.Ctxfor v3.import ( - "github.com/gofiber/fiber/v3" + "github.com/gofiber/fiber/v3" "github.com/gofiber/contrib/fiberzerolog" "github.com/rs/zerolog" + "os" ) @@ - app.Get("/", func (c *fiber.Ctx) error { + app.Get("/", func (c fiber.Ctx) error { return c.SendString("Hello, World!") })Also applies to: 65-66
33-45: Update Config type signatures for Fiber v3 (value-based Ctx).These entries still show pointer
*fiber.Ctx. For v3 they should accept/return withfiber.Ctx.-| Next | `func(*Ctx) bool` | Define a function to skip this middleware when returned true | `nil` | +| Next | `func(fiber.Ctx) bool` | Define a function to skip this middleware when returned true | `nil` | @@ -| GetLogger | `func(*fiber.Ctx) zerolog.Logger` | Get custom zerolog logger, if it's defined the returned logger will replace the `Logger` value. | `nil` | +| GetLogger | `func(fiber.Ctx) zerolog.Logger` | Get custom zerolog logger, if it's defined the returned logger will replace the `Logger` value. | `nil` | @@ -| GetResBody | func(c *fiber.Ctx) []byte | Define a function to get response body when return non-nil.<br />eg: When use compress middleware, resBody is unreadable. you can set GetResBody func to get readable resBody. | `nil` | +| GetResBody | `func(fiber.Ctx) []byte` | Define a function to get response body when return non-nil.<br />eg: When using compression middleware, resBody is unreadable. You can set GetResBody to return a readable body. | `nil` |fiberzerolog/config.go (1)
276-282: Guard against empty (non-nil) Messages/Levels to avoid panics.
If a user supplies an empty slice (not nil), indexing later will panic. Fill with defaults when len == 0.Apply:
- if cfg.Messages == nil { + if cfg.Messages == nil || len(cfg.Messages) == 0 { cfg.Messages = ConfigDefault.Messages } - if cfg.Levels == nil { + if cfg.Levels == nil || len(cfg.Levels) == 0 { cfg.Levels = ConfigDefault.Levels }fiberzerolog/zerolog.go (1)
75-83: Bug: passing fiber.Ctx to zerolog’s Event.Ctx — use std context via c.Context().
zerolog.Event.Ctx expects context.Context. In Fiber v3, use c.Context() (renamed from UserContext()).- ctx := c + ctx := c.Context()swagger/README.md (1)
100-106: Update Config.Next signature to Fiber v3 (value) Ctx.Docs still show a pointer. For v3, handlers/config callbacks take fiber.Ctx by value.
- Next func(c *fiber.Ctx) bool + Next func(c fiber.Ctx) boolopafiber/README.md (2)
39-45: Align types and config callbacks to Fiber v3 value Ctx.Docs use *fiber.Ctx; v3 uses fiber.Ctx by value. Update both the table default and the InputCreationFunc type.
-| InputCreationMethod | `InputCreationFunc` | Use your own function to provide input for OPA | `func defaultInput(ctx *fiber.Ctx) (map[string]interface{}, error)` | +| InputCreationMethod | `InputCreationFunc` | Use your own function to provide input for OPA | `func defaultInput(ctx fiber.Ctx) (map[string]interface{}, error)` |-type InputCreationFunc func(c *fiber.Ctx) (map[string]interface{}, error) +type InputCreationFunc func(c fiber.Ctx) (map[string]interface{}, error)
85-98: Example won’t compile: missing bytes import and pointer Ctx usage.Add bytes to imports and switch to value Ctx in the callback.
import ( - "github.com/gofiber/fiber/v3" - "github.com/gofiber/contrib/opafiber/v2" + "bytes" + "github.com/gofiber/fiber/v3" + "github.com/gofiber/contrib/opafiber/v2" ) @@ - InputCreationMethod: func (ctx *fiber.Ctx) (map[string]interface{}, error) { + InputCreationMethod: func (ctx fiber.Ctx) (map[string]interface{}, error) { return map[string]interface{}{ - "method": ctx.Method(), - "path": ctx.Path(), + "method": ctx.Method(), + "path": ctx.Path(), }, nil },otelfiber/README.md (2)
37-47: Update option signatures to Fiber v3 value Ctx.Docs show pointers; v3 uses fiber.Ctx by value.
-| `WithNext` | `func(*fiber.Ctx) bool` | Define a function to skip this middleware when returned true .| nil | +| `WithNext` | `func(fiber.Ctx) bool` | Define a function to skip this middleware when returned true. | nil | @@ -| `WithSpanNameFormatter` | `func(*fiber.Ctx) string` | Takes a function that will be called on every request and the returned string will become the span Name. | Default formatter returns the route pathRaw | +| `WithSpanNameFormatter` | `func(fiber.Ctx) string` | Takes a function that will be called on every request and the returned string will become the span Name. | Default formatter returns the route pathRaw | @@ -| `WithCustomAttributes` | `func(*fiber.Ctx) []attribute.KeyValue` | Define a function to add custom attributes to the span. | nil | +| `WithCustomAttributes` | `func(fiber.Ctx) []attribute.KeyValue` | Define a function to add custom attributes to the span. | nil | @@ -| `WithCustomMetricAttributes` | `func(*fiber.Ctx) []attribute.KeyValue` | Define a function to add custom attributes to the metrics. | nil | +| `WithCustomMetricAttributes` | `func(fiber.Ctx) []attribute.KeyValue` | Define a function to add custom attributes to the metrics. | nil |
96-100: Fix example: c.UserContext() → c.Context() (v3 rename) and JSON map key.In v3, UserContext() was renamed to Context(). Also make the JSON key a string literal.
- name := getUser(c.UserContext(), id) - return c.JSON(fiber.Map{"id": id, name: name}) + name := getUser(c.Context(), id) + return c.JSON(fiber.Map{"id": id, "name": name})fiberi18n/README.md (1)
26-31: *Signature table still shows fiber.Ctx; update to fiber.Ctx (v3)The codebase migrated to by-value Ctx; docs should match.
-| Localize | `Localize(ctx *fiber.Ctx, params interface{}) (string, error)` | Localize returns a localized message. param is one of these type: messageID, *i18n.LocalizeConfig | -| MustLocalize | `MustLocalize(ctx *fiber.Ctx, params interface{}) string` | MustLocalize is similar to Localize, except it panics if an error happens. param is one of these type: messageID, *i18n.LocalizeConfig | +| Localize | `Localize(ctx fiber.Ctx, params interface{}) (string, error)` | Localize returns a localized message. param is one of these type: messageID, *i18n.LocalizeConfig | +| MustLocalize | `MustLocalize(ctx fiber.Ctx, params interface{}) string` | MustLocalize is similar to Localize, except it panics if an error happens. param is one of these type: messageID, *i18n.LocalizeConfig |fibernewrelic/README.md (1)
37-39: Update Config function types to fiber.Ctx (v3)Docs still show *fiber.Ctx.
-| ErrorStatusCodeHandler | `func(c *fiber.Ctx, err error) int` | If you want to change newrelic status code, you can use it. | `DefaultErrorStatusCodeHandler` | -| Next | `func(c *fiber.Ctx) bool` | Next defines a function to skip this middleware when returned true. | `nil` | +| ErrorStatusCodeHandler | `func(c fiber.Ctx, err error) int` | If you want to change newrelic status code, you can use it. | `DefaultErrorStatusCodeHandler` | +| Next | `func(c fiber.Ctx) bool` | Next defines a function to skip this middleware when returned true. | `nil` |fibersentry/README.md (1)
56-106: Usefiber.Ctx(by value) in handlers and BeforeSend cast
- In fibersentry/README.md, change all handler signatures from
func(c *fiber.Ctx) errortofunc(c fiber.Ctx) errorto match the v3Handlerdefinition.- In the
BeforeSendexamples, replace the type assertionhint.Context.Value(...).(*fiber.Ctx)with.(fiber.Ctx)so you’re asserting the interface itself.casbin/README.md (4)
32-40: Docs still reference pointer-based Ctx; update to fiber.Ctx for v3The Config table should reflect by-value signatures in Fiber v3.
Apply:
-| Lookup | `func(*fiber.Ctx) string` | Look up for current subject | `""` | -| Unauthorized | `func(*fiber.Ctx) error` | Response body for unauthorized responses | `Unauthorized` | -| Forbidden | `func(*fiber.Ctx) error` | Response body for forbidden responses | `Forbidden` | +| Lookup | `func(fiber.Ctx) string` | Look up for current subject | `""` | +| Unauthorized | `func(fiber.Ctx) error` | Response body for unauthorized responses | `Unauthorized` | +| Forbidden | `func(fiber.Ctx) error` | Response body for forbidden responses | `Forbidden` |
63-66: Update example signatures to v3 (fiber.Ctxby value)Handlers and
Lookupstill use*fiber.Ctx.- Lookup: func(c *fiber.Ctx) string { + Lookup: func(c fiber.Ctx) string { ... - func(c *fiber.Ctx) error { + func(c fiber.Ctx) error { ... - func(c *fiber.Ctx) error { + func(c fiber.Ctx) error {Also applies to: 70-73, 77-80
104-107: RoutePermission example: switch to by-value Ctx- Lookup: func(c *fiber.Ctx) string { + Lookup: func(c fiber.Ctx) string { ... - func(c *fiber.Ctx) error { + func(c fiber.Ctx) error {Also applies to: 112-115
139-142: RoleAuthorization example: switch to by-value Ctx- Lookup: func(c *fiber.Ctx) string { + Lookup: func(c fiber.Ctx) string { ... - func(c *fiber.Ctx) error { + func(c fiber.Ctx) error {Also applies to: 146-149
fibersentry/sentry.go (1)
33-41: *Pass http.Request (not fiber.Ctx) into Sentry RecoverWithContext
RecoverWithContextexpects a context carrying the request viasentry.RequestContextKey. You already haverfromadaptor.ConvertRequest; use it to ensure proper request capture on panics.- eventID := hub.RecoverWithContext( - context.WithValue(context.Background(), sentry.RequestContextKey, c), - err, - ) + eventID := hub.RecoverWithContext( + context.WithValue(r.Context(), sentry.RequestContextKey, r), + err, + )jwt/utils.go (1)
19-31: Header extractor off-by-one can reject minimal valid tokens
len(auth) > l+1wrongly requires two extra chars beyond the scheme+space. Use> lso at least one char after the space is accepted.- if len(auth) > l+1 && strings.EqualFold(auth[:l], authScheme) { + if len(auth) > l && strings.EqualFold(auth[:l], authScheme) { return strings.TrimSpace(auth[l:]), nil }Optionally add a test for a single-char token to prevent regressions.
socketio/socketio_test.go (1)
132-166: WaitGroup leak on error paths → test can hang.Several early returns skip wg.Done(). Defer it at goroutine start and defer dial.Close() after a successful dial.
- for i := 0; i < numParallelTestConn; i++ { + for i := 0; i < numParallelTestConn; i++ { wg.Add(1) go func() { + defer wg.Done() dialer := &websocket.Dialer{ NetDial: func(network, addr string) (net.Conn, error) { return ln.Dial() }, HandshakeTimeout: 45 * time.Second, } dial, _, err := dialer.Dial(wsURL, nil) if err != nil { t.Error(err) return } + defer func() { + _ = dial.Close() + }() if err := dial.WriteMessage(websocket.TextMessage, []byte("test")); err != nil { t.Error(err) return } tp, m, err := dial.ReadMessage() if err != nil { t.Error(err) return } require.Equal(t, TextMessage, tp) require.Equal(t, "response", string(m)) - wg.Done() - - if err := dial.Close(); err != nil { - t.Error(err) - return - } }() }loadshed/loadshed.go (1)
16-22: Fix OnShed doc to match behavior
At loadshed/loadshed.go:19–20, the comment is incorrect—returning nil without callingc.Next()actually halts the chain. Update to:- // Returning `nil` without writing to the response context allows the - // request to proceed to the next handler + // To proceed, call and return `c.Next()`. + // Returning `nil` (without `c.Next()`) stops the handler chain.opafiber/fiber.go (1)
30-40: fmt.Sprint used with formatting verbs — error detail lost.fmt.Sprint doesn’t process %w; the panic message drops the underlying error.
- readedBytes, err := io.ReadAll(cfg.RegoPolicy) + policyBytes, err := io.ReadAll(cfg.RegoPolicy) if err != nil { - panic(fmt.Sprint("could not read rego policy %w", err)) + panic(fmt.Errorf("could not read rego policy: %w", err)) } query, err := rego.New( rego.Query(cfg.RegoQuery), - rego.Module("policy.rego", utils.UnsafeString(readedBytes)), + rego.Module("policy.rego", utils.UnsafeString(policyBytes)), ).PrepareForEval(context.Background()) if err != nil { - panic(fmt.Sprint("rego policy error: %w", err)) + panic(fmt.Errorf("rego policy error: %w", err)) }Also renames readedBytes → policyBytes for clarity.
loadshed/README.md (4)
36-64: Examples: update handlers to fiber.Ctx (value) for v3.
Current examples still use *fiber.Ctx.- app.Get("/", func(c *fiber.Ctx) error { + app.Get("/", func(c fiber.Ctx) error { return c.SendString("Welcome!") })
66-109: *Custom rejection example: update imports already v3, but OnShed/handlers still use fiber.Ctx.
Align to v3 and example style.- OnShed: func(ctx *fiber.Ctx) error { + OnShed: func(ctx fiber.Ctx) error { @@ - app.Get("/", func(c *fiber.Ctx) error { + app.Get("/", func(c fiber.Ctx) error { return c.SendString("Welcome!") })
115-120: Config table types out of date for v3 (pointer → value).-| Next | `func(*fiber.Ctx) bool` | Function to skip this middleware when returned true. | `nil` | +| Next | `func(fiber.Ctx) bool` | Function to skip this middleware when returned true. | `nil` | @@ -| OnShed | `func(c *fiber.Ctx) error` | Function to be executed if a request should be declined | `nil` | +| OnShed | `func(c fiber.Ctx) error` | Function to be executed if a request should be declined | `nil` |
147-149: Fix rejection probability formula (mixes percent and fraction).
Use normalized CPU value.- rejectionProbability := (cpuUsage - LowerThreshold*100) / (UpperThreshold - LowerThreshold) + rejectionProbability := (cpuUsage/100 - LowerThreshold) / (UpperThreshold - LowerThreshold)circuitbreaker/circuitbreaker.go (1)
238-244: Drain or reinitialize the half-open semaphore on transition.This drains at most one token. Leftover tokens from a prior half-open window reduce capacity unexpectedly. Reinitialize the channel to a fresh buffered chan.
- // Empty the semaphore channel - select { - case <-cb.halfOpenSemaphore: - default: - } + // Reset the semaphore to enforce fresh capacity for the new half-open window + cb.halfOpenSemaphore = make(chan struct{}, cb.config.HalfOpenMaxConcurrent)circuitbreaker/README.md (3)
56-60: Config table types don’t match v3 APIUpdate function types to use value-based
fiber.Ctxand includeCtxparameter forIsFailure.-| IsFailure | `func(error) bool` | Custom function to determine if an error is a failure | `Status >= 500` | -| OnOpen | `func(*fiber.Ctx)` | Callback function when the circuit is opened | `503 response` | -| OnClose | `func(*fiber.Ctx)` | Callback function when the circuit is closed | `Continue request` | -| OnHalfOpen | `func(*fiber.Ctx)` | Callback function when the circuit is half-open | `429 response` | +| IsFailure | `func(c fiber.Ctx, err error) bool` | Custom function to determine if an error is a failure | `Status >= 500 or err != nil` | +| OnOpen | `func(fiber.Ctx) error` | Callback when the circuit opens | `503 response` | +| OnClose | `func(fiber.Ctx) error` | Callback when the circuit closes | `Continue request` | +| OnHalfOpen | `func(fiber.Ctx) error` | Callback when the circuit is half-open | `429 response` |
75-83: Update examples to Fiber v3 (value Ctx, grouping, config fields, external calls)Multiple examples still use
*fiber.Ctx, an undefinedroute.Group, an outdatedHalfOpenSemaphorefield, andfiber.Get(non-existent client).Apply:
@@ -import ( - "github.com/gofiber/fiber/v3" - "github.com/gofiber/contrib/circuitbreaker" -) +import ( + "time" + "github.com/gofiber/fiber/v3" + "github.com/gofiber/contrib/circuitbreaker" +) @@ -app.Get("/", func(c *fiber.Ctx) error { +app.Get("/", func(c fiber.Ctx) error { return c.SendString("Hello, world!") }) @@ -app.Get("/metrics/circuit", func(c *fiber.Ctx) error { - return c.JSON(cb.GetStateStats()) +app.Get("/metrics/circuit", func(c fiber.Ctx) error { + return c.JSON(cb.GetStateStats()) }) @@ -app.Get("/protected", circuitbreaker.Middleware(cb), func(c *fiber.Ctx) error { +app.Get("/protected", circuitbreaker.Middleware(cb), func(c fiber.Ctx) error { return c.SendString("Protected service running") }) @@ -app := route.Group("/api") -app.Use(circuitbreaker.Middleware(cb)) +api := app.Group("/api") +api.Use(circuitbreaker.Middleware(cb)) @@ -app.Get("/users", getUsersHandler) -app.Post("/users", createUserHandler) +api.Get("/users", getUsersHandler) +api.Post("/users", createUserHandler) @@ -OnOpen: func(c *fiber.Ctx) error { +OnOpen: func(c fiber.Ctx) error { @@ -OnHalfOpen: func(c *fiber.Ctx) error { +OnHalfOpen: func(c fiber.Ctx) error { @@ -OnClose: func(c *fiber.Ctx) error { +OnClose: func(c fiber.Ctx) error { @@ -app.Get("/external-api", circuitbreaker.Middleware(cb), func(c *fiber.Ctx) error { - // Simulating an external API call - resp, err := fiber.Get("https://example.com/api") - if err != nil { - return fiber.NewError(fiber.StatusInternalServerError, "External API failed") - } - return c.SendString(resp.Body()) +app.Get("/external-api", circuitbreaker.Middleware(cb), func(c fiber.Ctx) error { + // Example using net/http client + resp, err := http.Get("https://example.com/api") + if err != nil { + return fiber.NewError(fiber.StatusBadGateway, "External API failed") + } + defer resp.Body.Close() + body, _ := io.ReadAll(resp.Body) + return c.Status(resp.StatusCode).Send(body) }) @@ -cb := circuitbreaker.New(circuitbreaker.Config{ - FailureThreshold: 3, - Timeout: 5 * time.Second, - SuccessThreshold: 2, - HalfOpenSemaphore: make(chan struct{}, 2), // Allow only 2 concurrent requests -}) +cb := circuitbreaker.New(circuitbreaker.Config{ + FailureThreshold: 3, + Timeout: 5 * time.Second, + SuccessThreshold: 2, + HalfOpenMaxConcurrent: 2, // Allow only 2 concurrent requests +}) @@ -app.Get("/db-service", circuitbreaker.Middleware(dbCB), func(c *fiber.Ctx) error { +app.Get("/db-service", circuitbreaker.Middleware(dbCB), func(c fiber.Ctx) error { @@ -app.Get("/api-service", circuitbreaker.Middleware(apiCB), func(c *fiber.Ctx) error { +app.Get("/api-service", circuitbreaker.Middleware(apiCB), func(c fiber.Ctx) error {Additionally, replace the shutdown snippet with a simpler, correct pattern:
defer cb.Stop() // ensure breaker stops on app exitAlso applies to: 93-113, 120-133, 140-161, 170-179, 188-199, 226-237
45-46: UpdateNewsignature in docs
Replace incircuitbreaker/README.md(lines 45–46):- circuitbreaker.New(config ...circuitbreaker.Config) *circuitbreaker.Middleware + circuitbreaker.New(config circuitbreaker.Config) *circuitbreaker.CircuitBreakerfibernewrelic/fiber.go (1)
73-81: Attach New Relic transaction to Fiber context
Insert immediately after starting the transaction:// Attach New Relic transaction to Fiber’s context for downstream retrieval // Fiber v3: use c.Context() (formerly UserContext) and SetContext() c.SetContext(newrelic.NewContext(c.Context(), txn))otelfiber/otelfiber_test/fiber_test.go (3)
370-401: Fix histogram helper: wrong slice increment and potential out-of-bounds.When value exceeds last bound you should increment the extra bucket in bucketCounts, not mutate bounds.
@@ - for i, v := range bounds { + for i, v := range bounds { if value <= v { bucketCounts[i]++ break } - if i == len(bounds)-1 { - bounds[i+1]++ - break - } + if i == len(bounds)-1 { + // value > last bound => increment the overflow bucket + bucketCounts[len(bounds)]++ + break + } }
553-561: Avoid setting global OTel provider inside parallel subtests.
otel.SetTracerProvideris global; calling it int.Parallel()subtests can race. You already inject the provider via middleware; remove the global set.- sr := tracetest.NewSpanRecorder() - provider := sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(sr)) - otel.SetTracerProvider(provider) + sr := tracetest.NewSpanRecorder() + provider := sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(sr))
496-515: Use the span’sctxfor propagation instead of the rawc
Replace inotelfiber/fiber.goaround line 175:- cfg.Propagators.Inject(c, tracingHeaders) + cfg.Propagators.Inject(ctx, tracingHeaders)so the
ctxreturned bytracer.Startis used for injection, ensuring the correct span context is propagated.fiberzap/README.md (1)
33-43: Config table types are outdated for v3 (value Ctx).Update types to match the v3 API:
Next,GetResBody,SkipBody,SkipResBody, andFieldsFuncshould acceptfiber.Ctx(by value), andFieldsFuncis a function, not[]zap.Field.-| Next | `func(*Ctx) bool` | Define a function to skip this middleware when returned true | `nil` | +| Next | `func(fiber.Ctx) bool` | Define a function to skip this middleware when returned true | `nil` | ... -| FieldsFunc | `[]zap.Field` | Define a function to add custom fields. | `nil` | +| FieldsFunc | `func(fiber.Ctx) []zap.Field` | Define a function to add custom fields. | `nil` | ... -| GetResBody | func(c \*fiber.Ctx) []byte | Define a function to get response body when return non-nil. | `nil` | +| GetResBody | `func(fiber.Ctx) []byte` | Define a function to get response body when return non-nil. | `nil` |fiberzap/zap.go (1)
112-114: Bug: loggingerr(always nil) instead of the actualchainErr.This drops error details from logs. Log
chainErr(when non-nil) or remove the unconditional error field to avoid duplication with the"error"case below.- fields := make([]zap.Field, 0, len(cfg.Fields)+1) - fields = append(fields, zap.Error(err)) + fields := make([]zap.Field, 0, len(cfg.Fields)+1) + if chainErr != nil { + fields = append(fields, zap.Error(chainErr)) + }fiberzap/logger.go (3)
98-105: Duplicate removal logic is a no-op; actually deduplicateExtraKeys.Looping over
cfg.ExtraKeysand checking membership in the same slice never removes dups. Build a set instead.- // Remove duplicated extraKeys - for _, k := range cfg.ExtraKeys { - if !contains(k, cfg.ExtraKeys) { - cfg.ExtraKeys = append(cfg.ExtraKeys, k) - } - } + // Deduplicate ExtraKeys + if len(cfg.ExtraKeys) > 1 { + seen := make(map[string]struct{}, len(cfg.ExtraKeys)) + uniq := make([]string, 0, len(cfg.ExtraKeys)) + for _, k := range cfg.ExtraKeys { + if _, ok := seen[k]; ok { + continue + } + seen[k] = struct{}{} + uniq = append(uniq, k) + } + cfg.ExtraKeys = uniq + }
129-145: Guard against emptyCoreConfigsto avoid panics inSetOutput.
l.CoreConfigs[0]panics if a user supplied an empty (non-nil) slice. Initialize a default core when empty.func (l *LoggerConfig) SetOutput(w io.Writer) { if l.SetLogger != nil { fiberlog.Warn("SetOutput is ignored when SetLogger is set") return } - l.CoreConfigs[0].WriteSyncer = zapcore.AddSync(w) + if len(l.CoreConfigs) == 0 { + l.CoreConfigs = []CoreConfig{ + { + Encoder: zapcore.NewJSONEncoder(zap.NewProductionEncoderConfig()), + WriteSyncer: zapcore.AddSync(w), + LevelEncoder: zap.NewAtomicLevelAt(zap.InfoLevel), + }, + } + } else { + l.CoreConfigs[0].WriteSyncer = zapcore.AddSync(w) + }
147-178: Same empty-slice guard needed inSetLevel.Avoid
l.CoreConfigs[0]whenCoreConfigsis empty.func (l *LoggerConfig) SetLevel(lv fiberlog.Level) { if l.SetLogger != nil { fiberlog.Warn("SetLevel is ignored when SetLogger is set") return } @@ - l.CoreConfigs[0].LevelEncoder = level + if len(l.CoreConfigs) == 0 { + l.CoreConfigs = []CoreConfig{ + { + Encoder: zapcore.NewJSONEncoder(zap.NewProductionEncoderConfig()), + WriteSyncer: zapcore.AddSync(os.Stdout), + LevelEncoder: level, + }, + } + } else { + l.CoreConfigs[0].LevelEncoder = level + }fiberi18n/embed_test.go (1)
51-53: Update App.Test call for Fiber v3.Fiber v3’s
App.Testtakes afiber.TestConfig. Pass an empty config for defaults.- resp, err := embedApp.Test(req) + resp, err := embedApp.Test(req, fiber.TestConfig{})websocket/websocket_test.go (1)
74-83: Potential panic: unconditionaldefer conn.Close()when Dial fails.In the “disallowed origin” case,
connis nil on handshake error; deferringconn.Close()will panic.Use a guarded cleanup:
conn, resp, err := websocket.DefaultDialer.Dial(url, hdr) if conn != nil { t.Cleanup(func() { _ = conn.Close() }) } assert.Error(t, err)Recommend applying the same pattern to all Dial sites.
fiberi18n/i18n.go (2)
75-84: Fix examples: use ctx.Params("name") instead of context.Param.
Public docs should reflect Fiber’s API; singular Param is incorrect.Apply:
- "name": context.Param("name"), + "name": ctx.Params("name"),
125-131: Validate params type; avoid passing nil LocalizeConfig.
If params is neither string nor *i18n.LocalizeConfig, localizeConfig stays nil; be explicit and return an error.switch paramValue := params.(type) { case string: localizeConfig = &i18n.LocalizeConfig{MessageID: paramValue} case *i18n.LocalizeConfig: localizeConfig = paramValue +default: + return "", fmt.Errorf("i18n.Localize error: unsupported params type %T", params) } message, err := localizer.(*i18n.Localizer).Localize(localizeConfig)Also applies to: 133-139
fiberi18n/i18n_test.go (1)
219-233: Do not call testing.T methods from goroutines.Using assert with t inside goroutines is unsafe and can race. Collect errors via channel and assert after wg.Wait, or spawn subtests with t.Run and t.Parallel.
Example (channel aggregation):
t.Run("test nil ctx", func(t *testing.T) { var wg sync.WaitGroup want := 100 + errs := make(chan error, want) wg.Add(want) for i := 0; i < want; i++ { go func() { defer wg.Done() got, err := makeRequest(language.English, "", app) - assert.Equal(t, nil, err) + if err != nil { errs <- err; return } + defer got.Body.Close() body, _ := io.ReadAll(got.Body) - assert.Equal(t, "en", string(body)) + if string(body) != "en" { errs <- fmt.Errorf("want en, got %q", body) } }() } wg.Wait() + close(errs) + for e := range errs { + assert.NoError(t, e) + } })jwt/README.md (2)
254-258: Remove or rewrite JWK Set section; field was removed in v3.Docs still reference
JWKSetURLs, which v3 removes per the migration notes/PR context. This will mislead users. Replace with v3-supported approach or delete. (docs.gofiber.io)-## JWK Set Test - -The tests are identical to basic `JWT` tests above, with exception that `JWKSetURLs` to valid public keys collection in JSON Web Key (JWK) Set format should be supplied. See [RFC 7517](https://www.rfc-editor.org/rfc/rfc7517). +<!-- JWK Set config was removed in v3. If/when JWK support returns, document the new configuration/API here. -->
260-306: Delete “Custom KeyFunc example” (removed in v3).
KeyFuncis no longer a public config field in v3. Keeping this section contradicts the v3 API and examples. (docs.gofiber.io)-## Custom KeyFunc example -... (entire section) +<!-- KeyFunc was removed in v3. If a new extension point exists, add an up-to-date example here. -->
| mapfile -t MODULE_DIRS < <( | ||
| find . \( -path "./.git" -o -path "./vendor" -o -path "./node_modules" \) -prune -o \ | ||
| -type f -name 'go.mod' -print0 | | ||
| xargs -0 -n1 dirname | | ||
| sort -u | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Broaden prunes to nested .git/vendor/node_modules.
Current patterns only skip top-level directories. Use globs to avoid scanning vendored or nested modules.
- find . \( -path "./.git" -o -path "./vendor" -o -path "./node_modules" \) -prune -o \
+ find . \( -path "*/.git" -o -path "*/vendor" -o -path "*/node_modules" \) -prune -o \
-type f -name 'go.mod' -print0 |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mapfile -t MODULE_DIRS < <( | |
| find . \( -path "./.git" -o -path "./vendor" -o -path "./node_modules" \) -prune -o \ | |
| -type f -name 'go.mod' -print0 | | |
| xargs -0 -n1 dirname | | |
| sort -u | |
| ) | |
| mapfile -t MODULE_DIRS < <( | |
| find . \( -path "*/.git" -o -path "*/vendor" -o -path "*/node_modules" \) -prune -o \ | |
| -type f -name 'go.mod' -print0 | | |
| xargs -0 -n1 dirname | | |
| sort -u | |
| ) |
🤖 Prompt for AI Agents
In .github/scripts/parallel-go-test.sh around lines 73 to 78, the find command
only prunes top-level .git, vendor and node_modules directories; update the
prune patterns to match nested directories (e.g. use globs/leading-wildcard
paths like */.git, */vendor, */node_modules) so find will skip any nested
vendored or git folders; ensure the new -path patterns are quoted and combined
with -o before -prune, and keep the rest of the pipeline (-type f -name 'go.mod'
-print0 | xargs -0 -n1 dirname | sort -u) unchanged.
| if ! grep -E -q '(--- FAIL:|^FAIL\b|panic:|exit status|FAIL\t|FAIL:)' "$src_log"; then | ||
| # nothing to do (only PASS/ok output) | ||
| rm -f "$src_log" >/dev/null 2>&1 || true | ||
| return | ||
| fi | ||
|
|
||
| local annotated | ||
| annotated="$(mktemp "$TMPDIR/annotated.XXXXXX")" || { | ||
| until mkdir "$lockdir" 2>/dev/null; do sleep 0.01; done | ||
| printf '==== %s ====\n' "$module_dir" >> "$LOGFILE" | ||
| # filter out PASS/OK noise when falling back | ||
| grep -v -E '^(=== RUN|--- PASS:|^PASS$|^ok\s)' "$src_log" >> "$LOGFILE" || true | ||
| rm -f "$src_log" || true |
There was a problem hiding this comment.
Fix regex usage: \s and \b aren’t supported here; may miss failures and skip lines incorrectly.
- grep -E doesn’t support
\bor\s. Use POSIX classes. - Bash
[[ … =~ … ]]also doesn’t support\s.
Apply:
- if ! grep -E -q '(--- FAIL:|^FAIL\b|panic:|exit status|FAIL\t|FAIL:)' "$src_log"; then
+ if ! grep -E -q '(--- FAIL:|^FAIL($|[[:space:]])|panic:|exit status|FAIL[[:space:]]|FAIL:)' "$src_log"; then
@@
- grep -v -E '^(=== RUN|--- PASS:|^PASS$|^ok\s)' "$src_log" >> "$LOGFILE" || true
+ grep -v -E '^(=== RUN|--- PASS:|PASS$|ok[[:space:]])' "$src_log" >> "$LOGFILE" || true
@@
- if [[ $line =~ ^(===\ RUN|---\ PASS:|^PASS$|^ok\s) ]]; then
+ if [[ $line =~ ^(===\ RUN|---\ PASS:|PASS$|ok[[:space:]]) ]]; then
continue
fiAlso applies to: 163-167
🤖 Prompt for AI Agents
In .github/scripts/parallel-go-test.sh around lines 145-157 (and similarly lines
163-167), the extended grep regex uses \b and \s which are unsupported by grep
-E; change those to POSIX character classes and explicit anchors. Replace
patterns like ^FAIL\b and FAIL\t with FAIL([[:space:]]|$) or
^FAIL([[:space:]]|$) (or simply FAIL[[:space:]]), and replace ^ok\s with
^ok[[:space:]]; update both the grep -E -q '(--- FAIL:|^FAIL\b|panic:|exit
status|FAIL\t|FAIL:)' and the grep -v -E '^(=== RUN|--- PASS:|^PASS$|^ok\s)'
calls to use the POSIX-style alternatives so grep -E correctly matches
whitespace and word boundaries.
| if [[ $line =~ ^([^:]+\.go):([0-9]+):?([0-9]*)[:[:space:]]*(.*)$ ]]; then | ||
| local fp="${BASH_REMATCH[1]}" | ||
| local ln="${BASH_REMATCH[2]}" | ||
| local col="${BASH_REMATCH[3]}" | ||
| local rest="${BASH_REMATCH[4]}" | ||
| local candidate="" |
There was a problem hiding this comment.
Correct file:line[:col] matcher; also use captured column (fixes ShellCheck SC2034).
- Current
[:[:space:]]*is malformed; column is captured but unused. - Emit
:colwhen available.
Apply:
- if [[ $line =~ ^([^:]+\.go):([0-9]+):?([0-9]*)[:[:space:]]*(.*)$ ]]; then
+ if [[ $line =~ ^([^:]+\.go):([0-9]+)(:([0-9]+))?[[:space:]]*(.*)$ ]]; then
local fp="${BASH_REMATCH[1]}"
local ln="${BASH_REMATCH[2]}"
- local col="${BASH_REMATCH[3]}"
- local rest="${BASH_REMATCH[4]}"
+ local col="${BASH_REMATCH[4]}"
+ local rest="${BASH_REMATCH[5]}"
@@
- if [ -n "$candidate" ]; then
+ if [ -n "$candidate" ]; then
local start=$(( ln > 1 ? ln - 1 : 1 ))
local end=$(( ln + 1 ))
- printf '%s\n' "---- source: $candidate:$ln ----" >> "$annotated"
+ if [ -n "$col" ]; then
+ printf '%s\n' "---- source: $candidate:$ln:$col ----" >> "$annotated"
+ else
+ printf '%s\n' "---- source: $candidate:$ln ----" >> "$annotated"
+ fi
awk -v s="$start" -v e="$end" 'NR>=s && NR<=e { printf("%6d %s\n", NR, $0) }' "$candidate" >> "$annotated"
printf '%s\n\n' "Error: $line" >> "$annotated"Also applies to: 185-191
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 173-173: col appears unused. Verify use (or export if used externally).
(SC2034)
🤖 Prompt for AI Agents
.github/scripts/parallel-go-test.sh around lines 170-175 (and similarly update
185-191): the regex character class is malformed and the captured column is
unused (triggering ShellCheck SC2034); change the pattern portion
`[:[:space:]]*` to the correct POSIX class `[[:space:]]*` and ensure you emit
the column when present by appending `:${col}` (only when col is non-empty) to
the file:line string so the captured `col` variable is actually used.
| OnClose: func(c fiber.Ctx) error { | ||
| return c.Next() | ||
| }, |
There was a problem hiding this comment.
Default OnClose should be a no-op; calling Next() here risks double-invocation.
Middleware already calls c.Next() before invoking OnClose. The default OnClose: c.Next() can cause “next called multiple times” or re-enter handlers.
- OnClose: func(c fiber.Ctx) error {
- return c.Next()
- },
+ OnClose: func(c fiber.Ctx) error {
+ return nil
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| OnClose: func(c fiber.Ctx) error { | |
| return c.Next() | |
| }, | |
| OnClose: func(c fiber.Ctx) error { | |
| return nil | |
| }, |
🤖 Prompt for AI Agents
In circuitbreaker/circuitbreaker.go around lines 59 to 61, the default OnClose
handler calls c.Next(), which can double-invoke the next handler because
Middleware already calls c.Next() before invoking OnClose; change the default to
a true no-op (do not call c.Next()) by replacing the function body to simply
return nil (or set OnClose to nil) so OnClose does not re-enter the handler
chain.
| // Default fgprof interval is 30 seconds | ||
| resp, err := app.Test(httptest.NewRequest("GET", "/debug/fgprof?seconds=1", nil), 1500) | ||
| utils.AssertEqual(t, nil, err) | ||
| utils.AssertEqual(t, 200, resp.StatusCode) | ||
| resp, err := app.Test(httptest.NewRequest("GET", "/debug/fgprof?seconds=1", nil), fiber.TestConfig{Timeout: 1500}) | ||
| assert.Equal(t, nil, err) | ||
| assert.Equal(t, 200, resp.StatusCode) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Timeout unit: confirm TestConfig.Timeout type and use duration if applicable
If fiber.TestConfig.Timeout is time.Duration in v3, pass 1500*time.Millisecond; if it’s int ms, current code is fine.
Suggested change if time.Duration:
- resp, err := app.Test(httptest.NewRequest("GET", "/debug/fgprof?seconds=1", nil), fiber.TestConfig{Timeout: 1500})
+ resp, err := app.Test(httptest.NewRequest("GET", "/debug/fgprof?seconds=1", nil), fiber.TestConfig{Timeout: 1500 * time.Millisecond})🏁 Script executed:
#!/bin/bash
# Check TestConfig.Timeout type in your workspace
rg -nP 'type\s+TestConfig\s+struct\s*{[^}]*\bTimeout\b' $(go env GOPATH 2>/dev/null)/pkg/mod -g '!**/vendor/**' -S -C2Length of output: 192
🏁 Script executed:
#!/bin/bash
# Search for TestConfig struct definition and its Timeout field type in this repository
rg -nP "type\s+TestConfig\s+struct" -C5 --type go .
rg -nP "TestConfig.*Timeout" -C3 --type go .Length of output: 13760
Use time.Duration for TestConfig.Timeout
TestConfig.Timeout is a time.Duration (1 = 1 ns) – passing 1500 sets 1500 ns, not 1.5 s; change to 1500 * time.Millisecond and import time. (docs.gofiber.io)
- resp, err := app.Test(httptest.NewRequest("GET", "/debug/fgprof?seconds=1", nil), fiber.TestConfig{Timeout: 1500})
+ resp, err := app.Test(httptest.NewRequest("GET", "/debug/fgprof?seconds=1", nil), fiber.TestConfig{Timeout: 1500 * time.Millisecond})Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In fgprof/fgprof_test.go around lines 56 to 60 the TestConfig.Timeout is being
passed as an integer literal 1500 which is interpreted as nanoseconds; change
the timeout to a time.Duration value (e.g. 1500 * time.Millisecond or
time.Second) and add an import for time at the top of the file so the test uses
the intended 1.5s timeout.
| assert.Equal(t, symmetricKey+symmetricKey, config.SymmetricKey) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify expected behavior for invalid symmetric key
Asserting key duplication is fine if configDefault enforces required length by repetition; confirm this logic matches paseto/config.go.
🏁 Script executed:
#!/bin/bash
# Inspect how SymmetricKey is normalized
rg -nP 'SymmetricKey|configDefault\(' paseto -C3 --type goLength of output: 8967
🏁 Script executed:
#!/bin/bash
# Inspect the default ConfigDefault declaration and its SymmetricKey value
rg -nP 'var\s+ConfigDefault' paseto/config.go -C10Length of output: 715
🏁 Script executed:
#!/bin/bash
rg -nP 'func configDefault' -C20 paseto/config.goLength of output: 1179
Test_Config_Invalid_SymmetricKey mismatch – configDefault panics on invalid SymmetricKey length rather than concatenating the key to meet chacha20poly1305.KeySize. Either remove the duplication assertion in paseto/config_test.go (lines 27–28) and only assert the panic, or add normalization logic to configDefault to repeat the key as the test expects.
🤖 Prompt for AI Agents
paseto/config_test.go lines 27-28: the test expects configDefault to normalize a
too-short SymmetricKey by repeating it to chacha20poly1305.KeySize, but
configDefault currently panics on invalid length; modify configDefault to
normalize keys instead of panicking — when SymmetricKey length is less than
chacha20poly1305.KeySize, repeat the key bytes (concatenate copies) and truncate
to exactly KeySize; when longer, truncate to KeySize; then remove the panic so
the test asserting symmetricKey+symmetricKey (or equivalent repeated key)
passes.
| | Property | Type | Description | Default | | ||
| |:---------------|:--------------------------------|:--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:--------------------------------| | ||
| | Next | `func(*Ctx) bool` | Defines a function to skip middleware | `nil` | | ||
| | SuccessHandler | `func(*fiber.Ctx) error` | SuccessHandler defines a function which is executed for a valid token. | `c.Next()` | | ||
| | ErrorHandler | `func(*fiber.Ctx, error) error` | ErrorHandler defines a function which is executed for an invalid token. | `401 Invalid or expired PASETO` | | ||
| | SuccessHandler | `func(fiber.Ctx) error` | SuccessHandler defines a function which is executed for a valid token. | `c.Next()` | | ||
| | ErrorHandler | `func(fiber.Ctx, error) error` | ErrorHandler defines a function which is executed for an invalid token. | `401 Invalid or expired PASETO` | | ||
| | Validate | `PayloadValidator` | Defines a function to validate if payload is valid. Optional. In case payload used is created using `CreateToken` function. If token is created using another function, this function must be provided. | `nil` | |
There was a problem hiding this comment.
Fix type in Config table: Next should use fiber.Ctx (by value).
The table still shows func(*Ctx) bool. It should be func(fiber.Ctx) bool to match v3 and the code.
-| Next | `func(*Ctx) bool` | Defines a function to skip middleware | `nil` |
+| Next | `func(fiber.Ctx) bool` | Defines a function to skip middleware | `nil` |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | Property | Type | Description | Default | | |
| |:---------------|:--------------------------------|:--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:--------------------------------| | |
| | Next | `func(*Ctx) bool` | Defines a function to skip middleware | `nil` | | |
| | SuccessHandler | `func(*fiber.Ctx) error` | SuccessHandler defines a function which is executed for a valid token. | `c.Next()` | | |
| | ErrorHandler | `func(*fiber.Ctx, error) error` | ErrorHandler defines a function which is executed for an invalid token. | `401 Invalid or expired PASETO` | | |
| | SuccessHandler | `func(fiber.Ctx) error` | SuccessHandler defines a function which is executed for a valid token. | `c.Next()` | | |
| | ErrorHandler | `func(fiber.Ctx, error) error` | ErrorHandler defines a function which is executed for an invalid token. | `401 Invalid or expired PASETO` | | |
| | Validate | `PayloadValidator` | Defines a function to validate if payload is valid. Optional. In case payload used is created using `CreateToken` function. If token is created using another function, this function must be provided. | `nil` | | |
| | Property | Type | Description | Default | | |
| |:---------------|:--------------------------------|:--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:--------------------------------| | |
| | Next | `func(fiber.Ctx) bool` | Defines a function to skip middleware | `nil` | | |
| | SuccessHandler | `func(fiber.Ctx) error` | SuccessHandler defines a function which is executed for a valid token. | `c.Next()` | | |
| | ErrorHandler | `func(fiber.Ctx, error) error` | ErrorHandler defines a function which is executed for an invalid token. | `401 Invalid or expired PASETO` | | |
| | Validate | `PayloadValidator` | Defines a function to validate if payload is valid. Optional. In case payload used is created using `CreateToken` function. If token is created using another function, this function must be provided. | `nil` | |
🧰 Tools
🪛 LanguageTool
[grammar] ~42-~42: There might be a mistake here.
Context: ... | 401 Invalid or expired PASETO | | Validate | PayloadValidator ...
(QB_NEW_EN)
[grammar] ~43-~43: There might be a mistake here.
Context: ...idate if payload is valid. Optional. In case payload used is created using `CreateTo...
(QB_NEW_EN)
[grammar] ~43-~43: There might be a mistake here.
Context: ...s created using CreateToken function. If token is created using another function...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In paseto/README.md around lines 38 to 43, the Config table incorrectly lists
Next as `func(*Ctx) bool`; update the table entry to `func(fiber.Ctx) bool` so
it matches v3 and the actual code signature. Edit only the table cell for Next's
Type to use the non-pointer fiber.Ctx type and ensure any surrounding formatting
remains intact.
| func restricted(c fiber.Ctx) error { | ||
| payload := pasetoware.FromContext(c).(string) | ||
| return c.SendString("Welcome " + payload) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use safe FromContext assertions in README examples to avoid panics.
Direct .() assertions will panic if middleware is bypassed/misconfigured. Show a defensive pattern in examples.
@@
-func restricted(c fiber.Ctx) error {
- payload := pasetoware.FromContext(c).(string)
- return c.SendString("Welcome " + payload)
-}
+func restricted(c fiber.Ctx) error {
+ v := pasetoware.FromContext(c)
+ payload, ok := v.(string)
+ if !ok {
+ return c.SendStatus(fiber.StatusUnauthorized)
+ }
+ return c.SendString("Welcome " + payload)
+}
@@
-func restricted(c fiber.Ctx) error {
- payload := pasetoware.FromContext(c).(customPayloadStruct)
- return c.SendString("Welcome " + payload.Name)
-}
+func restricted(c fiber.Ctx) error {
+ v := pasetoware.FromContext(c)
+ payload, ok := v.(customPayloadStruct)
+ if !ok {
+ return c.SendStatus(fiber.StatusUnauthorized)
+ }
+ return c.SendString("Welcome " + payload.Name)
+}
@@
-func restricted(c fiber.Ctx) error {
- payload := pasetoware.FromContext(c).(string)
- return c.SendString("Welcome " + payload)
-}
+func restricted(c fiber.Ctx) error {
+ v := pasetoware.FromContext(c)
+ payload, ok := v.(string)
+ if !ok {
+ return c.SendStatus(fiber.StatusUnauthorized)
+ }
+ return c.SendString("Welcome " + payload)
+}Also applies to: 242-245, 350-353
🤖 Prompt for AI Agents
In paseto/README.md around lines 128-131 (and similarly at 242-245, 350-353),
the README uses direct type assertions like payload :=
pasetoware.FromContext(c).(string) which can panic if the middleware is missing
or returns nil; change the examples to a defensive pattern: retrieve the value
with a comma-ok type assertion (or check for nil), handle the false/nil case by
returning an appropriate error or default response, and only use the asserted
value when safe so the example demonstrates resilient, non-panic-prone
middleware usage.
| resp, _ := app.Test(r, fiber.TestConfig{Timeout: 0, FailOnTimeout: false}) | ||
| return resp |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t ignore app.Test error.
Ignoring the error can mask real failures (router miss, panic). Panic is fine in a helper to keep call sites unchanged.
- resp, _ := app.Test(r, fiber.TestConfig{Timeout: 0, FailOnTimeout: false})
+ resp, err := app.Test(r, fiber.TestConfig{Timeout: 0, FailOnTimeout: false})
+ if err != nil {
+ panic(err)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| resp, _ := app.Test(r, fiber.TestConfig{Timeout: 0, FailOnTimeout: false}) | |
| return resp | |
| resp, err := app.Test(r, fiber.TestConfig{Timeout: 0, FailOnTimeout: false}) | |
| if err != nil { | |
| panic(err) | |
| } | |
| return resp |
🤖 Prompt for AI Agents
In swagger/swagger_test.go around lines 22 to 23, the helper currently ignores
the error returned by app.Test which can hide router misses or panics; modify
the helper to check the error returned by app.Test and panic if non-nil so
failures surface to tests. Call app.Test, capture (resp, err), if err != nil {
panic(err) }, then return resp.
| return func(c fiber.Ctx) error { | ||
| if cfg.Filter != nil && !cfg.Filter(c) { | ||
| return c.Next() | ||
| } | ||
|
|
||
| conn := acquireConn() |
There was a problem hiding this comment.
Filter logic inverted; requests won’t be skipped when Filter returns true.
Per comment “Filter defines a function to skip middleware”, skip when Filter(c) == true.
Apply:
- if cfg.Filter != nil && !cfg.Filter(c) {
- return c.Next()
- }
+ if cfg.Filter != nil && cfg.Filter(c) {
+ return c.Next()
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return func(c fiber.Ctx) error { | |
| if cfg.Filter != nil && !cfg.Filter(c) { | |
| return c.Next() | |
| } | |
| conn := acquireConn() | |
| return func(c fiber.Ctx) error { | |
| if cfg.Filter != nil && cfg.Filter(c) { | |
| return c.Next() | |
| } | |
| conn := acquireConn() |
🤖 Prompt for AI Agents
In websocket/websocket.go around lines 116 to 121, the filter logic is inverted:
currently it calls c.Next() when cfg.Filter(c) is false, but per the comment the
Filter function should skip the middleware when it returns true. Change the
condition to check cfg.Filter != nil && cfg.Filter(c) and return c.Next() in
that case (keep the nil check), so the middleware is skipped when Filter returns
true.
5 participants
Summary by CodeRabbit
New Features
Refactor
Documentation
Chores