🔥 feat: Add support for NewErrorf #3463
Conversation
WalkthroughA new function, Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant FiberApp
Caller->>FiberApp: NewErrorf(code, ...message)
alt No message arguments
FiberApp-->>Caller: Error{code, default status message}
else One argument
FiberApp-->>Caller: Error{code, stringified argument}
else Multiple arguments
alt First argument is string
FiberApp-->>Caller: Error{code, formatted message}
else
FiberApp-->>Caller: Error{code, stringified first argument}
end
end
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (5)
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3463 +/- ##
==========================================
+ Coverage 84.35% 84.42% +0.07%
==========================================
Files 120 120
Lines 12190 12213 +23
==========================================
+ Hits 10283 10311 +28
+ Misses 1477 1473 -4
+ Partials 430 429 -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:
|
gaby
changed the title
There was a problem hiding this comment.
Pull Request Overview
Adds support for variadic arguments in NewError for flexible message formatting, updates usage sites to the new signature, refreshes related documentation, and expands test coverage for formatting behaviors.
- Extend
NewErrorto accept...anyand format messages viafmt.Sprintf - Update
SendFileerror handling and documentation examples to the new API - Add comprehensive tests in
Test_NewError_Format
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/guide/error-handling.md | Updated example to use variadic format arguments |
| docs/api/fiber.md | Changed NewError signature to ...any and example |
| ctx.go | Replaced fmt.Sprintf call with direct NewError use |
| app.go | Implemented variadic overload logic in NewError |
| app_test.go | Added tests for various NewError formatting scenarios |
Comments suppressed due to low confidence (1)
app_test.go:1313
- [nitpick] It may be helpful to add a test case for a format string with no matching parameters (e.g.,
"missing %s"without args) to confirmNewError's behavior in that edge case.
// go test -run Test_NewError_Format
Co-authored-by: Copilot <[email protected]>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app.go(1 hunks)
🧰 Additional context used
🪛 GitHub Check: unit (1.24.x, ubuntu-latest)
app.go
[failure] 902-902:
undefined: msg
[failure] 898-898:
undefined: msg
[failure] 894-894:
undefined: msg
🪛 GitHub Check: unit (1.24.x, macos-latest)
app.go
[failure] 902-902:
undefined: msg
[failure] 898-898:
undefined: msg
[failure] 894-894:
undefined: msg
🪛 GitHub Check: unit (1.23.x, macos-latest)
app.go
[failure] 902-902:
undefined: msg
[failure] 898-898:
undefined: msg
[failure] 894-894:
undefined: msg
[failure] 902-902:
undefined: msg
[failure] 898-898:
undefined: msg
[failure] 894-894:
undefined: msg
🪛 GitHub Check: unit (1.23.x, ubuntu-latest)
app.go
[failure] 902-902:
undefined: msg
[failure] 898-898:
undefined: msg
[failure] 894-894:
undefined: msg
[failure] 902-902:
undefined: msg
[failure] 898-898:
undefined: msg
[failure] 894-894:
undefined: msg
🪛 GitHub Check: lint
app.go
[failure] 902-902:
undefined: msg (typecheck)
[failure] 898-898:
undefined: msg
[failure] 894-894:
undefined: msg
🪛 GitHub Check: govulncheck-check
app.go
[failure] 902-902:
undefined: msg
[failure] 898-898:
undefined: msg
[failure] 894-894:
undefined: msg
🪛 GitHub Actions: golangci-lint
app.go
[error] 894-894: golangci-lint error: undefined variable 'msg'
🪛 GitHub Actions: Run govulncheck
app.go
[error] 894-894: Undefined identifier 'msg' at line 894, column 3.
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Compare
- GitHub Check: Analyse
- GitHub Check: repeated
|
@gaby can we improve the performance ? |
gaby
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/whats_new.md (2)
3-3: Grammar: Title apostrophe
Consider updating the title to "🆕 What's New in v3" (adding the apostrophe) for correct English.
69-69: Include a link to theNewErrorfAPI docs and usage example
The newNewErrorfentry would benefit from a direct hyperlink to its detailed API documentation (e.g.,docs/api/fiber.md#NewErrorf) and a brief code snippet showing its signature and usage. This aligns with other method entries and improves discoverability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/api/fiber.md(1 hunks)docs/whats_new.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/api/fiber.md
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: unit (1.23.x, macos-13)
- GitHub Check: unit (1.24.x, windows-latest)
- GitHub Check: unit (1.23.x, macos-latest)
- GitHub Check: unit (1.23.x, windows-latest)
- GitHub Check: repeated
- GitHub Check: Compare
Summary
NewErrorvia new function called:NewErrorfRelated to #3455