-
Notifications
You must be signed in to change notification settings - Fork 555
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
lambda: check if error type is InvokeResponse_Error #312
Conversation
Currently, we cannot set the errorType field without creating a custom named error type. This causes a proliferation of dummy named error types whose only purpose is to set a text field, and limits the errorType field to what can be expressed as an identifier. This change checks that the panicked or returned error is if type InvokeResponse_Error, and if so, uses it directly in the invoker's response. InvokeResponse_Error is now updated to implement error. Note: This is fully backwards compatible, since InvokeResponse_Error previously did not implement the error interface. Fixes aws#308.
Codecov Report
@@ Coverage Diff @@
## master #312 +/- ##
==========================================
- Coverage 72.63% 72.23% -0.41%
==========================================
Files 18 18
Lines 720 724 +4
==========================================
Hits 523 523
- Misses 134 136 +2
- Partials 63 65 +2
Continue to review full report at Codecov.
|
Thanks for the PR @smasher164 - Until now, I've considered the types in the I've forwarded this to the team to check for alternative approaches to the problem. |
Hey @bmoffatt, friendly ping to ask if there's any word on the issue/PR?
type Error struct{
Message string `json:"errorMessage"`
Type string `json:"errorType"`
}
type Error interface {
error
Type() string
} The disadvantage is that the second option is a potentially breaking change, if the user's error implements the Type method. |
I talked with the team, and we figured that if we ever want to get rid of the The |
Currently, we cannot set the errorType field without creating a custom
named error type. This causes a proliferation of dummy named error types
whose only purpose is to set a text field, and limits the errorType
field to what can be expressed as an identifier.
This change checks that the panicked or returned error is if type
InvokeResponse_Error, and if so, uses it directly in the invoker's
response. InvokeResponse_Error is now updated to implement error.
Note: This is fully backwards compatible, since InvokeResponse_Error
previously did not implement the error interface.
Fixes #308.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.