🧹 chore: Refactor internal errors to use sentinel values#3864
🧹 chore: Refactor internal errors to use sentinel values#3864ReneWerner87 merged 3 commits intomainfrom
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. WalkthroughReplaces many inline errors with package-level sentinel error variables across multiple packages, adds new error declaration files, and updates call sites and tests to reference these predeclared errors instead of constructing errors with errors.New(). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (1)**/*.go📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (4)📚 Learning: 2024-10-08T19:06:06.583ZApplied to files:
📚 Learning: 2024-07-01T03:44:03.672ZApplied to files:
📚 Learning: 2024-10-08T19:06:06.583ZApplied to files:
📚 Learning: 2024-10-08T19:06:06.583ZApplied to files:
🧬 Code graph analysis (1)middleware/encryptcookie/encryptcookie_test.go (1)
⏰ 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). (6)
🔇 Additional comments (3)
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 |
Summary of ChangesHello @gaby, 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 significantly refactors the internal error handling mechanisms within the Fiber framework. The primary goal is to move away from ad-hoc error creation using Highlights
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
|
There was a problem hiding this comment.
Code Review
This pull request is a great improvement, consistently refactoring inline errors.New calls to use sentinel errors across the codebase. This enhances error handling robustness and maintainability. The introduction of dedicated error definition files and the corresponding updates to tests using require.ErrorIs are excellent practices. I have one suggestion to further improve error context in one of the modified files.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3864 +/- ##
==========================================
+ Coverage 91.67% 91.69% +0.02%
==========================================
Files 115 115
Lines 9819 9819
==========================================
+ Hits 9002 9004 +2
+ Misses 517 516 -1
+ Partials 300 299 -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
This PR refactors error handling across the Fiber codebase by replacing inline errors.New() calls with package-level sentinel error variables, improving error comparison capabilities using errors.Is() and enabling better error handling patterns.
- Introduces dedicated error definition files (
errors_internal.go,client/errors.go,middleware/logger/errors.go) to centralize error declarations - Replaces inline error construction with sentinel error variables in core fiber pools and type assertions
- Updates test assertions to use
require.ErrorIs()instead ofrequire.Error()for better error validation
Reviewed Changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| errors_internal.go | New file defining internal sentinel errors for type assertions and escape sequences in the main fiber package |
| client/errors.go | New file defining sentinel errors for client package type assertions and sync pool operations |
| middleware/logger/errors.go | New file defining ErrTemplateParameterMissing for logger template validation |
| middleware/static/static.go | Introduces ErrInvalidPath sentinel error and replaces inline error construction |
| middleware/static/static_test.go | Updates test to use ErrorIs assertion for sentinel error validation |
| middleware/encryptcookie/utils.go | Adds ErrInvalidEncryptedValue sentinel error to existing error definitions |
| middleware/basicauth/config.go | Adds ErrInvalidSHA256PasswordLength sentinel error |
| middleware/adaptor/adaptor.go | Adds ErrRemoteAddrEmpty and ErrRemoteAddrTooLong sentinel errors |
| middleware/adaptor/adaptor_test.go | Updates tests to validate sentinel errors using ErrorIs assertions |
| middleware/logger/template_chain.go | Wraps error with fmt.Errorf to include context when template parameter is missing |
| internal/tlstest/tls.go | Introduces errAppendCACert sentinel error for CA certificate operations |
| binder/binder.go | Adds ErrInvalidDestinationValue, ErrUnmatchedBrackets, and errPoolTypeAssertion |
| binder/mapping.go | Replaces inline errors with sentinel errors for bracket parsing and destination validation |
| binder/mapping_test.go | Updates tests to use ErrorIs assertions for sentinel error validation |
| bind.go | Replaces inline error with errBindPoolTypeAssertion sentinel error |
| ctx_interface.go | Replaces inline error with errCustomCtxTypeAssertion sentinel error |
| helpers.go | Replaces inline errors with errTLSConfigTypeAssertion and errInvalidEscapeSequence |
| req.go | Replaces inline error with errTCPAddrTypeAssertion sentinel error |
| redirect.go | Replaces inline error with errRedirectTypeAssertion sentinel error |
| client/core.go | Replaces inline errors with sentinel errors for channel type assertions |
| client/request.go | Replaces inline errors with sentinel errors for Request and File type assertions |
| client/hooks.go | Replaces inline error with errSyncPoolBuffer sentinel error |
| client/cookiejar.go | Replaces inline error with errCookieJarTypeAssertion sentinel error |
| req_interface_gen.go | Formatting change: moves nolint comment to its own line |
| ctx_interface_gen.go | Formatting change: moves nolint comment to its own line |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
internal/tlstest/tls.go (1)
18-18: Consider clarifying the error message.The sentinel error approach is correct and aligns with the PR objectives. However, the error message "append CA cert to cert pool" could be more explicit about the failure. Consider using "failed to append CA cert to cert pool" for clarity.
Apply this diff if you'd like to improve the message:
-var errAppendCACert = errors.New("append CA cert to cert pool") +var errAppendCACert = errors.New("failed to append CA cert to cert pool")middleware/adaptor/adaptor_test.go (1)
1123-1177: Consider removing the unusedexpectErrorfield.The
expectErrorfield in the test struct is never read. Line 1184 recomputes the error expectation fromexpectedErranderrorContains, shadowing the struct field. Removing this field would eliminate confusion.Apply this diff to remove the unused field:
tests := []struct { expectedErr error localAddr any name string remoteAddr string errorContains string - expectError bool }{ { name: "valid TCP address with port", remoteAddr: "192.168.1.1:8080", localAddr: nil, - expectError: false, }, { name: "valid TCP address without port - should add default port 80", remoteAddr: "192.168.1.1", localAddr: nil, - expectError: false, }, { name: "unix socket - should return local addr", remoteAddr: "irrelevant", localAddr: &net.UnixAddr{Name: "/tmp/test.sock", Net: "unix"}, - expectError: false, }, { name: "invalid address - should fail", remoteAddr: "[invalid:address:format", localAddr: nil, - expectError: true, errorContains: "failed to resolve TCP address:", }, { name: "invalid address after adding port - should fail", remoteAddr: "[invalid", localAddr: nil, - expectError: true, errorContains: "failed to resolve TCP address after adding port:", }, { name: "empty address - should fail", remoteAddr: "", localAddr: nil, - expectError: true, expectedErr: ErrRemoteAddrEmpty, }, { name: "too long address - should fail", remoteAddr: strings.Repeat("a", 254), localAddr: nil, - expectError: true, expectedErr: ErrRemoteAddrTooLong, }, }binder/binder.go (2)
13-14: Consider consistency in error message format.
ErrUnmatchedBracketsis missing the"binder:"prefix that all other exported errors in this file use (lines 10-12). For consistency, consider:- ErrUnmatchedBrackets = errors.New("unmatched brackets") + ErrUnmatchedBrackets = errors.New("binder: unmatched brackets")
17-17: Improve error message clarity.The error message mentions the literal string "T", which is the generic type parameter name. This could be confusing to users who see this in a panic message, as they won't know what "T" refers to.
-var errPoolTypeAssertion = errors.New("failed to type-assert to T") +var errPoolTypeAssertion = errors.New("failed to type-assert pool value to the requested type")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
bind.go(1 hunks)binder/binder.go(2 hunks)binder/mapping.go(3 hunks)binder/mapping_test.go(2 hunks)client/cookiejar.go(1 hunks)client/core.go(2 hunks)client/errors.go(1 hunks)client/hooks.go(1 hunks)client/request.go(2 hunks)ctx_interface.go(2 hunks)ctx_interface_gen.go(1 hunks)errors_internal.go(1 hunks)helpers.go(2 hunks)internal/tlstest/tls.go(2 hunks)middleware/adaptor/adaptor.go(3 hunks)middleware/adaptor/adaptor_test.go(3 hunks)middleware/basicauth/config.go(2 hunks)middleware/encryptcookie/utils.go(2 hunks)middleware/logger/errors.go(1 hunks)middleware/logger/template_chain.go(2 hunks)middleware/static/static.go(4 hunks)middleware/static/static_test.go(1 hunks)redirect.go(1 hunks)req.go(1 hunks)req_interface_gen.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Apply formatting using gofumpt (Make target: format)
Optimize struct field alignment using betteralign (Make target: betteralign)
Modernize code using gopls modernize (Make target: modernize)
Files:
req.gomiddleware/basicauth/config.goclient/request.goclient/hooks.gohelpers.goredirect.gointernal/tlstest/tls.goclient/cookiejar.gomiddleware/static/static.gobinder/mapping.gomiddleware/adaptor/adaptor_test.gomiddleware/static/static_test.gomiddleware/encryptcookie/utils.goclient/core.gomiddleware/adaptor/adaptor.gobinder/binder.gomiddleware/logger/errors.gobinder/mapping_test.goctx_interface.goclient/errors.goctx_interface_gen.gobind.gomiddleware/logger/template_chain.goerrors_internal.goreq_interface_gen.go
🧠 Learnings (14)
📚 Learning: 2024-10-16T12:12:30.506Z
Learnt from: gaby
Repo: gofiber/fiber PR: 3170
File: ctx_test.go:1721-1724
Timestamp: 2024-10-16T12:12:30.506Z
Learning: In the Go unit tests in `ctx_test.go`, it is acceptable to use invalid CIDR notation such as `"0.0.0.1/31junk"` for testing purposes.
Applied to files:
middleware/adaptor/adaptor_test.go
📚 Learning: 2025-10-16T07:19:52.418Z
Learnt from: grivera64
Repo: gofiber/fiber PR: 3807
File: adapter_test.go:118-144
Timestamp: 2025-10-16T07:19:52.418Z
Learning: In the Fiber codebase, the linter does not allow `require` assertions from within HTTP handlers (including net/http-style handlers). Use `t.Fatalf`, `t.Errorf`, or similar `testing.T` methods for error handling inside handler functions instead.
Applied to files:
middleware/static/static_test.gomiddleware/adaptor/adaptor.goctx_interface.goclient/errors.gobind.gomiddleware/logger/template_chain.goerrors_internal.go
📚 Learning: 2024-12-13T08:14:22.851Z
Learnt from: efectn
Repo: gofiber/fiber PR: 3162
File: hooks_test.go:228-228
Timestamp: 2024-12-13T08:14:22.851Z
Learning: In Go test files, prefer using the `require` methods from the `testify` package for assertions instead of manual comparisons and calls to `t.Fatal` or `t.Fatalf`.
Applied to files:
middleware/static/static_test.gobinder/mapping_test.go
📚 Learning: 2024-11-15T07:56:21.623Z
Learnt from: ReneWerner87
Repo: gofiber/fiber PR: 3161
File: app.go:923-932
Timestamp: 2024-11-15T07:56:21.623Z
Learning: In the Fiber framework, breaking changes are acceptable when moving from version 2 to version 3, including modifications to method signatures such as in the `Test` method in `app.go`.
Applied to files:
middleware/static/static_test.go
📚 Learning: 2024-10-08T19:06:06.583Z
Learnt from: gaby
Repo: gofiber/fiber PR: 3056
File: middleware/encryptcookie/utils.go:22-25
Timestamp: 2024-10-08T19:06:06.583Z
Learning: The `encryptcookie_test.go` file contains unit tests that validate key lengths for both `EncryptCookie` and `DecryptCookie` functions, ensuring that invalid key lengths raise appropriate errors.
Applied to files:
middleware/encryptcookie/utils.go
📚 Learning: 2024-07-01T03:44:03.672Z
Learnt from: gaby
Repo: gofiber/fiber PR: 3056
File: middleware/encryptcookie/utils.go:20-23
Timestamp: 2024-07-01T03:44:03.672Z
Learning: Unit tests for key length enforcement in both `EncryptCookie` and `DecryptCookie` functions have been added to ensure robust validation and prevent potential runtime errors.
Applied to files:
middleware/encryptcookie/utils.go
📚 Learning: 2024-10-08T19:06:06.583Z
Learnt from: gaby
Repo: gofiber/fiber PR: 3056
File: middleware/encryptcookie/utils.go:51-54
Timestamp: 2024-10-08T19:06:06.583Z
Learning: Unit tests for key length enforcement in `DecryptCookie` have been added to ensure consistency and security in the encryption processes.
Applied to files:
middleware/encryptcookie/utils.go
📚 Learning: 2024-10-02T23:02:12.306Z
Learnt from: sixcolors
Repo: gofiber/fiber PR: 3016
File: middleware/session/session.go:46-61
Timestamp: 2024-10-02T23:02:12.306Z
Learning: In this codebase, the `sessionPool` only contains `Session` instances, so type assertions without additional checks are acceptable.
Applied to files:
client/core.gobinder/binder.goerrors_internal.go
📚 Learning: 2025-10-16T07:15:26.529Z
Learnt from: grivera64
Repo: gofiber/fiber PR: 3807
File: adapter_test.go:118-144
Timestamp: 2025-10-16T07:15:26.529Z
Learning: In Fiber v3, net/http handlers (http.Handler, http.HandlerFunc, or raw func(http.ResponseWriter, *http.Request)) can be passed directly to routing methods like app.Get(), app.Post(), etc. The framework automatically detects and wraps them internally via toFiberHandler/collectHandlers. The github.com/gofiber/fiber/v3/middleware/adaptor package is legacy and should not be suggested for tests or code using native net/http handler support.
Applied to files:
middleware/adaptor/adaptor.goctx_interface.go
📚 Learning: 2024-11-10T23:44:13.704Z
Learnt from: gaby
Repo: gofiber/fiber PR: 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:
ctx_interface.go
📚 Learning: 2024-11-08T04:10:42.990Z
Learnt from: gaby
Repo: gofiber/fiber PR: 3193
File: middleware/cache/cache_test.go:897-897
Timestamp: 2024-11-08T04:10:42.990Z
Learning: In the Fiber framework, `Context()` is being renamed to `RequestCtx()`, and `UserContext()` to `Context()` to improve clarity and align with Go's context conventions.
Applied to files:
ctx_interface.go
📚 Learning: 2024-10-08T19:06:06.583Z
Learnt from: sixcolors
Repo: gofiber/fiber PR: 3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the Fiber framework tests, using `ctx.Response.Header.Cookie` may not be suitable for parsing cookies from the response header, as it requires a `*Cookie` and fills it rather than returning a string value; thus, manual parsing of the `Set-Cookie` header may be necessary.
Applied to files:
ctx_interface.go
📚 Learning: 2024-09-25T16:18:34.719Z
Learnt from: sixcolors
Repo: gofiber/fiber PR: 3016
File: middleware/session/config.go:122-122
Timestamp: 2024-09-25T16:18:34.719Z
Learning: In `DefaultErrorHandler(c *fiber.Ctx, err error)`, since `c` is a pointer to an interface, we need to dereference `*c` when calling interface methods like `SendStatus`.
Applied to files:
ctx_interface.go
📚 Learning: 2024-10-08T19:06:06.583Z
Learnt from: sigmundxia
Repo: gofiber/fiber PR: 3151
File: middleware/adaptor/adaptor_test.go:226-230
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In Fiber, the `c.Set` method does not return an error.
Applied to files:
bind.goerrors_internal.go
🧬 Code graph analysis (7)
binder/mapping.go (1)
binder/binder.go (2)
ErrInvalidDestinationValue(13-13)ErrUnmatchedBrackets(14-14)
middleware/adaptor/adaptor_test.go (1)
middleware/adaptor/adaptor.go (2)
ErrRemoteAddrEmpty(41-41)ErrRemoteAddrTooLong(42-42)
middleware/static/static_test.go (1)
middleware/static/static.go (1)
ErrInvalidPath(22-22)
middleware/encryptcookie/utils.go (1)
middleware/encryptcookie/encryptcookie.go (1)
New(11-58)
binder/binder.go (1)
app.go (1)
New(523-636)
binder/mapping_test.go (1)
binder/binder.go (1)
ErrUnmatchedBrackets(14-14)
middleware/logger/template_chain.go (1)
middleware/logger/errors.go (1)
ErrTemplateParameterMissing(6-6)
⏰ 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). (7)
- GitHub Check: Agent
- GitHub Check: lint
- GitHub Check: Compare
- GitHub Check: unit (1.25.x, macos-latest)
- GitHub Check: update_release_draft
- GitHub Check: unit (1.25.x, windows-latest)
- GitHub Check: repeated
🔇 Additional comments (28)
middleware/logger/errors.go (1)
1-6: LGTM! Well-structured sentinel error definition.The sentinel error follows Go conventions with an exported name prefixed with
Err, a clear error message with package context, and proper documentation. This enables callers to useerrors.Is()for error inspection while maintaining a consistent error value.middleware/logger/template_chain.go (2)
5-5: LGTM! Appropriate import change.The
fmtpackage import is correctly added to support the error wrapping withfmt.Errorfat line 50.
50-50: LGTM! Excellent error wrapping implementation.The change properly wraps
ErrTemplateParameterMissingwith contextual information using%wand%qverbs. This enables callers to useerrors.Is(err, ErrTemplateParameterMissing)for error inspection while providing the problematic template substring as additional debugging context.internal/tlstest/tls.go (1)
127-129: LGTM!The sentinel error is used correctly when
AppendCertsFromPEMfails, successfully replacing the inline error construction with the standardized error variable.middleware/static/static_test.go (1)
1178-1178: LGTM! Proper use of sentinel error testing.The update to use
require.ErrorIsis the correct and idiomatic way to test for the sentinel errorErrInvalidPath. This is more robust than string comparison and allows for proper error wrapping.middleware/static/static.go (2)
22-22: LGTM! Proper sentinel error declaration.The exported sentinel error
ErrInvalidPathfollows Go conventions and enables callers to useerrors.Is()for type-safe error checking.
48-48: LGTM! Consistent use of sentinel error.All three error return sites in
sanitizePathcorrectly use the sentinelErrInvalidPathinstead of inline error construction. This is more efficient (avoids allocations) and enables type-safe error checking.Also applies to: 58-58, 69-69
middleware/basicauth/config.go (2)
18-19: LGTM! Well-structured sentinel error.The exported error variable follows Go conventions and aligns with the PR's objective to standardize error handling. The descriptive name makes the error's purpose clear, and exporting it enables callers to use
errors.Is()for type-safe error checking.
193-193: LGTM! Correct sentinel error usage.The change correctly replaces inline error construction with the sentinel error. This is appropriate here since it's a validation error (length check) rather than wrapping an underlying decoding error, and it enables type-safe error checking for callers.
middleware/adaptor/adaptor_test.go (1)
1184-1193: LGTM: Proper sentinel error verification.The test correctly verifies sentinel errors using
errors.IswhenexpectedErris set, and properly asserts thataddris nil when an error occurs. This follows Go best practices for testing sentinel errors.middleware/adaptor/adaptor.go (2)
40-43: LGTM: Well-defined sentinel errors.The sentinel error definitions follow Go conventions with clear, descriptive messages. Exporting these errors allows callers to use
errors.Is()for precise error handling, which is a best practice.
185-197: LGTM: Appropriate sentinel error usage.The function correctly returns sentinel errors for well-defined failure modes (empty address and address too long), while continuing to wrap unexpected errors with additional context. The 253-byte limit is accurate for maximum hostname length in DNS.
helpers.go (2)
71-71: LGTM - Sentinel error improves consistency.The replacement of inline error construction with
errTLSConfigTypeAssertionis appropriate for this type assertion failure path.
405-413: LGTM - Sentinel error enables better error handling.Replacing inline error construction with
errInvalidEscapeSequenceallows callers to useerrors.Is()for precise error checking.req_interface_gen.go (1)
33-34: LGTM - Formatting improvement.Moving the nolint directive to its own line improves readability without changing functionality.
client/core.go (2)
236-236: LGTM - Sentinel error for pool type assertion.The panic with
errResponseChanTypeAssertionappropriately handles the type assertion failure from the sync.Pool.
261-261: LGTM - Consistent error handling.Using
errErrChanTypeAssertionmaintains consistency with other pool acquisitions in this file.client/hooks.go (1)
267-267: LGTM - Sentinel error for pool buffer type assertion.Using
errSyncPoolBufferprovides a consistent error value for type assertion failures and allows proper error handling by callers.client/cookiejar.go (1)
25-25: LGTM - Consistent with pool acquisition pattern.The sentinel error
errCookieJarTypeAssertionaligns with the established pattern for sync.Pool type assertion failures across the client package.bind.go (1)
43-43: LGTM - Standardized error handling.Using
errBindPoolTypeAssertionmaintains consistency with the pool acquisition pattern established throughout the PR.client/request.go (2)
986-986: LGTM - Sentinel error for request pool.Using
errRequestTypeAssertionprovides consistent error handling for the request pool acquisition.
1037-1037: LGTM - Sentinel error for file pool.The
errFileTypeAssertionsentinel error maintains consistency with other pool acquisition functions in this file.binder/mapping.go (2)
110-110: LGTM - Exported sentinel error for validation.Using
ErrInvalidDestinationValueallows callers to check for this specific error condition usingerrors.Is(). The exported nature is appropriate for a validation error that external code may need to handle.
181-181: LGTM - Consistent error handling for bracket parsing.Replacing inline errors with
ErrUnmatchedBracketsin both locations provides a consistent, testable error value for bracket mismatch conditions.Also applies to: 192-192
client/errors.go (1)
5-12: LGTM! Clean sentinel error definitions.The error variables are well-defined with clear, descriptive messages. Using unexported variables is appropriate here since these errors are used for internal type-assertion failures in panic scenarios.
binder/mapping_test.go (1)
93-93: LGTM! Proper use of sentinel errors in tests.The test has been correctly updated to use the
ErrUnmatchedBracketssentinel error and properly checks it withrequire.ErrorIs, which is the correct approach for comparing sentinel errors.Also applies to: 98-98, 103-103, 135-135
binder/binder.go (1)
81-88: LGTM! Proper use of sentinel error in panic.The function now uses the predefined
errPoolTypeAssertionsentinel error instead of constructing a new error inline, which aligns well with the PR's objective of centralizing error definitions.middleware/encryptcookie/utils.go (1)
14-17: LGTM! Well-structured sentinel errors.The error declarations follow Go best practices:
- Exported for public API use with
errors.Is()checks- Clear, actionable error messages
- Logical grouping in a single var block
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
| Benchmark suite | Current: 72b9f66 | Previous: 6c8eccc | Ratio |
|---|---|---|---|
Benchmark_Compress_Levels/Zstd_LevelBestCompression - B/op |
1 B/op |
0 B/op |
+∞ |
This comment was automatically generated by workflow using github-action-benchmark.
Summary
errors.Newconstruction in core fiber pools with shared sentinel errors