Specific response assertion error messages #547
Merged
Conversation
This file contains 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
PietervdWerk
force-pushed
the
specific-type-assertion-errors
branch
from
1851450 to
bbc9d14
Compare
anderseknert
reviewed
May 3, 2024
envoyauth/response.go
Outdated
| @@ -307,7 +307,7 @@ func (result *EvalResult) GetDynamicMetadata() (*_structpb.Struct, error) { | |||
|
|
|||
| metadata, ok := val.(map[string]interface{}) | |||
| if !ok { | |||
| return nil, fmt.Errorf("type assertion error") | |||
| return nil, fmt.Errorf("type assertion error, dynamic_metadata is not a map[string]interface{}") | |||
There was a problem hiding this comment.
map[string]interface{} is a Go type, not a Rego one. I'd just say "is not an object" here. Same for headers below.
PietervdWerk
force-pushed
the
specific-type-assertion-errors
branch
2 times, most recently
from
8ff459d to
4a935df
Compare
|
I've updated the error messages to be "expected x, got y". Let me me know what you think :). |
anderseknert
previously approved these changes
May 8, 2024
ashutosh-narkar
force-pushed
the
specific-type-assertion-errors
branch
from
4a935df to
c55648b
Compare
There was a problem hiding this comment.
@PietervdWerk the changes lgtm. It would be great if we can update the tests to reflect these detailed error messages.
|
@PietervdWerk this would be a good change to get in. Are you able to update the tests? |
|
Yes tomorrow! (I had a few days off, back to work tomorrow.) |
Signed-off-by: Pieter van der Werk <pieter.vdwerk@gmail.com>
Signed-off-by: Pieter van der Werk <pieter.vdwerk@gmail.com>
ashutosh-narkar
force-pushed
the
specific-type-assertion-errors
branch
from
c969be8 to
33de40b
Compare
ashutosh-narkar
approved these changes
May 14, 2024
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.
I've been getting the error
"failed to get response body: type assertion error"and it took me quite a while to figure out what the problem was. I've copied my policy into a playground and it worked fine there (in hindsight that's obvious).I propose a more verbose and specific error message when assertion fails. I've taken the liberty to come with some suggestions.