docs: update documentation for parser configuration and request handling#4096
docs: update documentation for parser configuration and request handling#4096ReneWerner87 merged 4 commits intomainfrom
Conversation
Summary of ChangesHello @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 primarily focuses on enhancing and clarifying the documentation across various core components. It includes important corrections to code examples, improved descriptions for parser configurations and request handling, and updates to reflect recent API changes, ensuring the documentation accurately represents the current state of the application. Highlights
Changelog
Activity
Using Gemini Code AssistThe 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 by creating a comment using either
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 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
|
WalkthroughThis PR updates docs and binding docs: clarifies multipart file-field binding, moves parser config types to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4096 +/- ##
==========================================
+ Coverage 90.99% 91.03% +0.03%
==========================================
Files 119 119
Lines 11337 11320 -17
==========================================
- Hits 10316 10305 -11
+ Misses 648 643 -5
+ Partials 373 372 -1
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:
|
There was a problem hiding this comment.
Pull request overview
Updates Fiber v3 documentation to better reflect current parser/binding configuration and request/response handling APIs, plus a small inline comment correction in Bind.Form.
Changes:
- Align
whats_new.mdexamples/signatures with v3 APIs (handlers returningerror,SendStreamWriter/Drop/Endreturningerror, use offiber.Ctxin examples). - Fix/clarify config documentation (
IdleTimeout/ReadTimeout/WriteTimeoutdefaults as0, correctMsgPackEncoder/Decodernaming and types). - Clarify binding behavior/docs around parser decoder configuration and when request body participates in
Bind().All(), and updateBind.Formcomment about multipart file support.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| docs/whats_new.md | Updates migration notes and code samples to match v3 signatures and types. |
| docs/api/fiber.md | Corrects config table entries (timeouts default values; MsgPack encoder/decoder naming/types/defaults). |
| docs/api/bind.md | Adds binding-source clarification and updates parser decoder configuration docs (but see review comment on signature accuracy). |
| docs/api/app.md | Fixes TestConfig.Timeout example to use time.Second (not time.Second()). |
| bind.go | Updates Bind.Form comment to reflect multipart file header binding support. |
![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
The pull request correctly updates the documentation to reflect changes in Fiber v3, specifically regarding the new binding system, parser configuration, and updated method signatures that now return errors. It also fixes several typos in code examples (e.g., time.Second() to time.Second) and improves consistency in default value descriptions for durations and encoders. I have identified one minor inconsistency in the fiber.md configuration table where the * prefix was removed for MsgPack default values but retained for CBOR, despite both being function values.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/whats_new.md (1)
724-751:⚠️ Potential issue | 🟡 Minor
c.End()error silently discarded after the signature now returns one.Line 724 correctly documents the new
errorreturn forEnd(), but the example at line 747 silently drops the error:c.End() // Closes the underlying connection return errors.New("No name provided")Readers following this example will learn to discard the error return. The example should either handle it or explicitly discard it with
_:🔧 Suggested fix
- c.SendString("You don't have a name?") - c.End() // Closes the underlying connection + _ = c.SendString("You don't have a name?") + _ = c.End() // Closes the underlying connection; errors intentionally ignored return errors.New("No name provided")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/whats_new.md` around lines 724 - 751, Update the example showing Ctx.End() to not silently ignore its error return: in the example handler that calls c.End() (method Ctx.End), either check and handle the returned error (e.g., capture it and log or wrap it before returning) or explicitly discard it using the blank identifier (e.g., _ = c.End()). Modify the snippet inside the app.Get("/hello"...) example so the call to c.End() is followed by an explicit error handling or `_ = c.End()` and keep the subsequent `return errors.New("No name provided")` behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@docs/whats_new.md`:
- Around line 724-751: Update the example showing Ctx.End() to not silently
ignore its error return: in the example handler that calls c.End() (method
Ctx.End), either check and handle the returned error (e.g., capture it and log
or wrap it before returning) or explicitly discard it using the blank identifier
(e.g., _ = c.End()). Modify the snippet inside the app.Get("/hello"...) example
so the call to c.End() is followed by an explicit error handling or `_ =
c.End()` and keep the subsequent `return errors.New("No name provided")`
behavior.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/whats_new.md (1)
499-502:⚠️ Potential issue | 🟡 MinorPre-existing compile error in the Test Config example —
returnwith a value in a void handler.The handler at line 500 is declared as
func(c fiber.Ctx)(no return type), but line 501 usesreturn c.SendString("hello world"), which attempts to return anerrorvalue from a void function. This is a compile error in Go, independent of the handler-compatibility table's Case 2 support.🐛 Proposed fix
-app.Get("/", func(c fiber.Ctx) { - return c.SendString("hello world") -}) +app.Get("/", func(c fiber.Ctx) error { + return c.SendString("hello world") +})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/whats_new.md` around lines 499 - 502, The handler used in the Test Config example is declared as func(c fiber.Ctx) (no return) but uses return c.SendString("hello world"), causing a compile error; change the handler signature to accept and return an error (e.g., func(c *fiber.Ctx) error) or remove the return and call c.SendString without returning, and ensure you use the correct ctx type (fiber.Ctx vs *fiber.Ctx) consistent with app.Get's expected handler (reference app, fiber.New, app.Get, the handler func signature, and c.SendString).
🧹 Nitpick comments (2)
docs/api/bind.md (1)
640-668: Consider adding thebinderpackage import path to the example.The example uses
binder.SetParserDecoder,binder.ParserConfig, andbinder.ParserType— a package-level import that is not obvious to readers. Unlike handler-scoped calls, this function is typically called frommain()orinit(), so readers need to know the import path. Adding animportblock at the top of the example would make it self-contained.✏️ Suggested addition to the Example block
import ( "reflect" "time" "github.com/gofiber/fiber/v3/binder" )The API rename from
fiber.*tobinder.*in the usage and type instantiation (lines 659, 665, 667) is correct and matches thebinderpackage's exportedSetParserDecoderfunction.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/api/bind.md` around lines 640 - 668, The example is missing the import block for the binder package and other used packages; add an import statement including "github.com/gofiber/fiber/v3/binder" plus "reflect" and "time" (and any other standard packages used) at the top of the snippet so the usages of binder.SetParserDecoder, binder.ParserConfig, and binder.ParserType and the CustomTime/time.Parse calls compile and the example is self-contained.docs/whats_new.md (1)
746-747: LGTM — intentional_ =error-ignore pattern is appropriate here.The
c.End()comment is clear. Consider adding a matching inline comment to line 746 (// response already ended below) so both ignored errors are self-documented at the call site.✏️ Optional: symmetric comments
- _ = c.SendString("You don't have a name?") - _ = c.End() // Closes the underlying connection; errors intentionally ignored + _ = c.SendString("You don't have a name?") // errors intentionally ignored; connection is closed below + _ = c.End() // Closes the underlying connection; errors intentionally ignored🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/whats_new.md` around lines 746 - 747, Add a symmetric inline comment to the c.SendString call to document the intentional ignored error like the c.End() line does; update the call to c.SendString (in the same block that calls c.End()) to append a short comment such as "// response already ended below" so both ignored "_ = ..." calls (c.SendString and c.End) clearly explain why their errors are intentionally discarded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/api/bind.md`:
- Around line 621-622: The Signature block shows a bare call expression instead
of a Go function declaration; replace the line
"binder.SetParserDecoder(parserConfig binder.ParserConfig)" with a proper
function signature declaration "func SetParserDecoder(parserConfig
ParserConfig)" (i.e., add the "func" keyword, use the local type name
ParserConfig rather than binder.ParserConfig) so the Signature section correctly
documents the SetParserDecoder function and references ParserConfig.
---
Outside diff comments:
In `@docs/whats_new.md`:
- Around line 499-502: The handler used in the Test Config example is declared
as func(c fiber.Ctx) (no return) but uses return c.SendString("hello world"),
causing a compile error; change the handler signature to accept and return an
error (e.g., func(c *fiber.Ctx) error) or remove the return and call
c.SendString without returning, and ensure you use the correct ctx type
(fiber.Ctx vs *fiber.Ctx) consistent with app.Get's expected handler (reference
app, fiber.New, app.Get, the handler func signature, and c.SendString).
---
Nitpick comments:
In `@docs/api/bind.md`:
- Around line 640-668: The example is missing the import block for the binder
package and other used packages; add an import statement including
"github.com/gofiber/fiber/v3/binder" plus "reflect" and "time" (and any other
standard packages used) at the top of the snippet so the usages of
binder.SetParserDecoder, binder.ParserConfig, and binder.ParserType and the
CustomTime/time.Parse calls compile and the example is self-contained.
In `@docs/whats_new.md`:
- Around line 746-747: Add a symmetric inline comment to the c.SendString call
to document the intentional ignored error like the c.End() line does; update the
call to c.SendString (in the same block that calls c.End()) to append a short
comment such as "// response already ended below" so both ignored "_ = ..."
calls (c.SendString and c.End) clearly explain why their errors are
intentionally discarded.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/api/bind.md`:
- Around line 590-592: The documentation line describing WithAutoHandling has a
run-on sentence and missing punctuation; update the sentence "This function does
NOT panic therefore you must still return on error explicitly" to include a
comma after "panic" and end the sentence with a period (e.g., "This function
does NOT panic, therefore you must still return on error explicitly."); ensure
the WithAutoHandling description line also ends with a trailing period.
---
Duplicate comments:
In `@docs/api/bind.md`:
- Around line 620-622: The Signature block incorrectly uses a qualified type
name; change the parameter type in the SetParserDecoder signature from
binder.ParserConfig to the unqualified ParserConfig (i.e., update the function
signature for SetParserDecoder to accept ParserConfig) so it follows Go doc
conventions for types within the same package.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/whats_new.md (1)
734-751:⚠️ Potential issue | 🟡 Minor
log.Printlndoesn't process format verbs —%vwill appear literally.Line 736 in the same
Endexample block useslog.Println("Got error: %v", err).log.Printlndoes not support format specifiers; the output will beGot error: %v <error_string>instead of the intended interpolated form.🐛 Proposed fix
- log.Println("Got error: %v", err) + log.Printf("Got error: %v\n", err)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/whats_new.md` around lines 734 - 751, The log call inside the middleware using log.Println("Got error: %v", err) is incorrect because log.Println doesn't process format verbs; update the middleware's logging to either use log.Printf("Got error: %v", err) or change to log.Println("Got error:", err) so the error value is logged correctly — locate the anonymous middleware passed to app.Use and replace the log.Println call accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@docs/whats_new.md`:
- Around line 734-751: The log call inside the middleware using log.Println("Got
error: %v", err) is incorrect because log.Println doesn't process format verbs;
update the middleware's logging to either use log.Printf("Got error: %v", err)
or change to log.Println("Got error:", err) so the error value is logged
correctly — locate the anonymous middleware passed to app.Use and replace the
log.Println call accordingly.
1 participant
No description provided.