feat: make validation error messages customizable #520
Conversation
WalkthroughThe recent changes enhance the validation framework by standardizing error message handling across multiple files. Hardcoded strings have been replaced with predefined constants, improving maintainability and clarity. A new file centralizes various validation error messages, ensuring consistent and informative feedback. Overall, these adjustments lead to a more organized and reliable approach to error reporting in schema validation. Changes
Sequence Diagram(s)sequenceDiagram
participant Schema
participant Validator
participant ErrorFormatter
Schema->>Validator: Validate Schema
Validator->>ErrorFormatter: Format Error Messages
ErrorFormatter->>Validator: Return Formatted Message
Validator->>Schema: Validation Result
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- schema.go (3 hunks)
- validate.go (15 hunks)
- validation/messages.go (1 hunks)
Additional comments not posted (9)
validation/messages.go (2)
5-47: LGTM! Well-defined error message constants.The predefined error message constants improve maintainability and consistency across the validation framework.
49-50: LGTM! Customizable error formatter function.The error formatter function enhances flexibility by allowing customizable error message formatting.
validate.go (6)
284-284: LGTM! Improved maintainability and consistency.The use of predefined error message constants improves maintainability and consistency in the
validateAnyOffunction.
Line range hint
328-475:
LGTM! Improved maintainability and consistency.The use of predefined error message constants improves maintainability and consistency in the
validatefunction.
497-510: LGTM! Improved maintainability and consistency.The use of predefined error message constants improves maintainability and consistency in the
handleArrayfunction.
597-597: LGTM! Improved maintainability and consistency.The use of predefined error message constants improves maintainability and consistency in the
handleMapStringfunction.
697-697: LGTM! Improved maintainability and consistency.The use of predefined error message constants improves maintainability and consistency in the
handleMapAnyfunction.
187-249: LGTM! Improved maintainability and consistency.The use of predefined error message constants and the customizable error formatter improves maintainability and consistency in the
validateFormatfunction.However, ensure that all function calls to
validateFormatmatch the new implementation.Verification successful
Verification successful.
All instances of
validateFormatin the codebase match the new implementation.
validate.go: The only call tovalidateFormatis within this file and aligns with the new implementation.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `validateFormat` match the new implementation. # Test: Search for the function usage. Expect: Only occurances of the new implementation. rg --type go -A 5 $'validateFormat'Length of output: 615
Script:
#!/bin/bash # Description: Search for all instances of `validateFormat` across the entire codebase to ensure consistency with the new implementation. # Test: Search for all function calls to `validateFormat`. Expect: Only occurrences of the new implementation. rg 'validateFormat\('Length of output: 168
schema.go (1)
Line range hint
207-270:
LGTM! Improved consistency and maintainability.The use of the new
validation.ErrorFormatterfunction for various validation checks improves consistency and maintainability in thePrecomputeMessagesfunction.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #520 +/- ##
==========================================
- Coverage 92.83% 92.82% -0.02%
==========================================
Files 22 22
Lines 3838 3832 -6
==========================================
- Hits 3563 3557 -6
Misses 230 230
Partials 45 45 ☔ View full report in Codecov by Sentry. |
validation/messages.go
Outdated
| ) | ||
|
|
||
| // ErrorFormatter is a function that formats an error message | ||
| var ErrorFormatter func(format string, a ...any) string = fmt.Sprintf |
There was a problem hiding this comment.
Random thought: what about making this aware of the request context to enable i18n/l10n scenarios? For example, you could then use a library to return a localized error message based on the request's Accept-Language header. As is with your PR you could provide a single langauge, what I'm suggesting here is providing many languages and choosing one based on the request's preferences.
There was a problem hiding this comment.
Actually it is the next change I want to submit :) But decided to split the work into smaller pieces.
I noticed you try to avoid strings allocation during request validation. Using ctx would break it because we won't be able to do pre-computation.
The current approach I'm trying is to define 2 interfaces:
// ErrorFormatter is a function that formats an error message
var ErrorFormatter func(format string, a ...any) string = fmt.Sprintf
// ErrorFormatterContext is a function that will be used instead of ErrorFormatter
// to format an error message when defined
var ErrorFormatterContext func(ctx Context, format string, a ...any) string = nilUsage of ErrorFormatterContext is opt-in. I didn't do benchmarking yet though. If you don't mind I'll continue with it in the following weeks.
There was a problem hiding this comment.
I also moved it to the root of huma. It would make code easier to read as we introduce context.
There was a problem hiding this comment.
Good point about the pre-allocation. This probably requires a bit more thought, so I'm happy to get this improvement in first!
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
validation/messages.go (1)
1-45: Ensure consistent usage of validation error message constants.The codebase uses the defined constants in
validation/messages.goconsistently. However, there are still some hardcoded validation error messages present in the test files. It is recommended to replace these hardcoded strings with the defined constants to maintain consistency.
- Files with hardcoded strings:
validate_test.goschema_test.goqueryparam_test.gohuma_test.goPlease update these test files to use the constants from
validation/messages.go.Analysis chain
LGTM! Ensure consistent usage of constants.
The code changes are approved.
However, verify that these constants are used consistently throughout the codebase to avoid hardcoded strings.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of validation error message constants. # Test: Search for the usage of each constant. Expect: No hardcoded strings for validation error messages. rg --type go -A 5 $'MsgUnexpectedProperty|MsgExpectedRFC3339DateTime|MsgExpectedRFC1123DateTime|MsgExpectedRFC3339Date|MsgExpectedRFC3339Time|MsgExpectedRFC5322Email|MsgExpectedRFC5890Hostname|MsgExpectedRFC2673IPv4|MsgExpectedRFC2373IPv6|MsgExpectedRFC3986URI|MsgExpectedRFC4122UUID|MsgExpectedRFC6570URITemplate|MsgExpectedRFC6901JSONPointer|MsgExpectedRFC6901RelativeJSONPointer|MsgExpectedRegexp|MsgExpectedMatchSchema|MsgExpectedNotMatchSchema|MsgExpectedBoolean|MsgExpectedNumber|MsgExpectedString|MsgExpectedBase64String|MsgExpectedArray|MsgExpectedObject|MsgExpectedArrayItemsUnique|MsgExpectedOneOf|MsgExpectedMinimumNumber|MsgExpectedExclusiveMinimumNumber|MsgExpectedMaximumNumber|MsgExpectedExclusiveMaximumNumber|MsgExpectedNumberBeMultipleOf|MsgExpectedMinLength|MsgExpectedMaxLength|MsgExpectedBePattern|MsgExpectedMatchPattern|MsgExpectedMinItems|MsgExpectedMaxItems|MsgExpectedMinProperties|MsgExpectedMaxProperties|MsgExpectedRequiredProperty|MsgExpectedDependentRequiredProperty'Length of output: 13084
Script:
#!/bin/bash # Description: Verify there are no hardcoded validation error messages in the codebase. # Test: Search for hardcoded strings that match the patterns of the validation error messages. rg --type go -F "unexpected property" -F "expected string to be RFC 3339 date-time" -F "expected string to be RFC 1123 date-time" -F "expected string to be RFC 3339 date" -F "expected string to be RFC 3339 time" -F "expected string to be RFC 5322 email" -F "expected string to be RFC 5890 hostname" -F "expected string to be RFC 2673 ipv4" -F "expected string to be RFC 2373 ipv6" -F "expected string to be RFC 3986 uri" -F "expected string to be RFC 4122 uuid" -F "expected string to be RFC 6570 uri-template" -F "expected string to be RFC 6901 json-pointer" -F "expected string to be RFC 6901 relative-json-pointer" -F "expected string to be regex" -F "expected value to match at least one schema but matched none" -F "expected value to not match schema" -F "expected boolean" -F "expected number" -F "expected string" -F "expected string to be base64 encoded" -F "expected array" -F "expected object" -F "expected array items to be unique" -F "expected value to be one of" -F "expected number >=" -F "expected number >" -F "expected number <=" -F "expected number <" -F "expected number to be a multiple of" -F "expected length >=" -F "expected length <=" -F "expected string to be" -F "expected string to match pattern" -F "expected array length >=" -F "expected array length <=" -F "expected object with at least" -F "expected object with at most" -F "expected required property" -F "expected property to be present when"Length of output: 4182
Script:
#!/bin/bash # Description: Verify there are no hardcoded validation error messages in the codebase. # Test: Search for hardcoded strings that match the patterns of the validation error messages. rg --type go -e "unexpected property" -e "expected string to be RFC 3339 date-time" -e "expected string to be RFC 1123 date-time" -e "expected string to be RFC 3339 date" -e "expected string to be RFC 3339 time" -e "expected string to be RFC 5322 email" -e "expected string to be RFC 5890 hostname" -e "expected string to be RFC 2673 ipv4" -e "expected string to be RFC 2373 ipv6" -e "expected string to be RFC 3986 uri" -e "expected string to be RFC 4122 uuid" -e "expected string to be RFC 6570 uri-template" -e "expected string to be RFC 6901 json-pointer" -e "expected string to be RFC 6901 relative-json-pointer" -e "expected string to be regex" -e "expected value to match at least one schema but matched none" -e "expected value to not match schema" -e "expected boolean" -e "expected number" -e "expected string" -e "expected string to be base64 encoded" -e "expected array" -e "expected object" -e "expected array items to be unique" -e "expected value to be one of" -e "expected number >=" -e "expected number >" -e "expected number <=" -e "expected number <" -e "expected number to be a multiple of" -e "expected length >=" -e "expected length <=" -e "expected string to be" -e "expected string to match pattern" -e "expected array length >=" -e "expected array length <=" -e "expected object with at least" -e "expected object with at most" -e "expected required property" -e "expected property to be present when"Length of output: 10802
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- schema.go (3 hunks)
- validate.go (15 hunks)
- validation/messages.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- schema.go
- validate.go
smacker
force-pushed
the
allow-override-errors
branch
from
5ece0ad to
71fac32
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- error.go (1 hunks)
- schema.go (3 hunks)
- validate.go (14 hunks)
- validate_test.go (1 hunks)
- validation/messages.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- schema.go
- validate.go
Additional comments not posted (2)
error.go (1)
366-367: LGTM! Ensure proper usage ofErrorFormatter.The new variable
ErrorFormatteris correctly defined and initialized withfmt.Sprintf. This enhances the flexibility of error message formatting.However, ensure that all usages of
fmt.Sprintffor error messages are updated to useErrorFormatterif applicable.validate_test.go (1)
1303-1323: LGTM! Ensure comprehensive test coverage.The new test function
TestValidateCustomFormatteris correctly defined and effectively tests the custom error formatter by setting a custom formatter and validating an email format.However, ensure that other validation scenarios are also covered by tests to comprehensively test the custom error formatter.
I added a test for the new code but I believe it won't improve the report from codecov. I would really appreciate if you could help me to understand how to resolve this issue. |
Hello!
In the project I'm working on we show validation errors directly to the end user.
In this project we need to return them in specific format defined by the product.
But currently validation error messages are hard-coded in the source code so we would need to either replace validation completely or rely on existing text messages that might change over time.
This PR tries to improve it by allowing:
For example we want to replace existing error:
with something simple like
the following code let us do it
I'm not sure if we need unit tests for this and what exactly we need to test.
Please let me know if the idea resonates with you or you have something else in mind. Thank you!
Summary by CodeRabbit
New Features
ErrorFormatterfunction to enhance error message formatting dynamically.Bug Fixes
Tests