Fixes panic in UnwrapError #5
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If an error were passed to eal.UnwrapError that wasn't hashable/comparable, a panic were raised when we tried to use it as a map key. Now we check in UnwrapError if the error is comparable before we try to use it as a map key. If one tries to call RegisterErrorLogFunc with an error that isn't comparable we'll still raise a panic since until we can log those, there is no point to call RegisterErrorLogFunc, and that will in most cases happen when initializing the program. The only workaround currently is to either ignore log augmentation for those errors, or change the type to use pointer-receivers to force the creation of those errors to return a pointer to the un-comparable error struct. This commit also fixes some comments and another bug in the internal esl.errorLogger where the "jwt_text" log field contained the error bitfield and not the jwt.ValidationError text.
joeledstrom
reviewed
Mar 17, 2023
error_test.go
Outdated
| want: map[string]interface{}{"error_message": "testErr", "registeredErrorLogFunctions": true, "set_log_fields": true, "temporary": false, "type_*eal.testErr": true}, | ||
| }, | ||
| { | ||
| name: "ApiError-pointer", |
There was a problem hiding this comment.
rename test to nonComparableError-pointer or similar
There was a problem hiding this comment.
Ah, yes, good catch, I missed that when I re-created a minimal error struct that isn't hashable/comparable. I'll fix both occurrences.
joeledstrom
reviewed
Mar 17, 2023
error_test.go
Outdated
| want: map[string]interface{}{"error_message": "test,lines", "registeredErrorLogFunctions": true, "type_*eal.nonComparableError": true}, | ||
| }, | ||
| { | ||
| name: "ApiError-struct", |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If an error were passed to eal.UnwrapError that wasn't hashable/comparable, a panic were raised when we tried to use it as a map key. Now we check in UnwrapError if the error is comparable before we try to use it as a map key.
If one tries to call RegisterErrorLogFunc with an error that isn't comparable we'll still raise a panic since until we can log those, there is no point to call RegisterErrorLogFunc, and that will in most cases happen when initializing the program.
The only workaround currently is to either ignore log augmentation for those errors, or change the type to use pointer-receivers to force the creation of those errors to return a pointer to the un-comparable error struct.
This commit also fixes some comments and another bug in the internal esl.errorLogger where the "jwt_text" log field contained the error bitfield and not the jwt.ValidationError text.