Skip to content
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

detect if returned error is of type InvokeResponse_Error #308

Closed
smasher164 opened this issue Aug 4, 2020 · 1 comment · Fixed by #312
Closed

detect if returned error is of type InvokeResponse_Error #308

smasher164 opened this issue Aug 4, 2020 · 1 comment · Fixed by #312

Comments

@smasher164
Copy link
Contributor

smasher164 commented Aug 4, 2020

Is your feature request related to a problem? Please describe.
We currently check the errorType field to determine the category of errors to which a message corresponds. However, the errorType field is set by reflecting on the error value's concrete type, which leads to an influx of errorString or other error types that show up. Since, Go doesn't support creating a named type (golang/go#16522), we need some way of setting a custom errorType value.

Describe the solution you'd like
The lambda handler already constructs an InvokeResponse_Error, after reflecting on the user-returned error. Just type-assert that the error's underlying concrete type is InvokeResponse_Error, and if so, use that. This will allow us to directly set its Type field, as necessary.

Describe alternatives you've considered
Currently, we work around this by predeclaring a whole host of "dummy" error wrapper types, like so:

type categoryA struct{ error }
func (e categoryA) Error() string   { return e.error.Error() }
type categoryB struct{ error }
func (e categoryB) Error() string   { return e.error.Error() }
type categoryC struct{ error }
func (e categoryC) Error() string   { return e.error.Error() }
type categoryD struct{ error }
func (e categoryD) Error() string   { return e.error.Error() }
...
@smasher164
Copy link
Contributor Author

Note that this is also backwards compatible, since InvokeResponse_Error doesn't currently implement Error.

smasher164 added a commit to smasher164/aws-lambda-go that referenced this issue Aug 11, 2020
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.
bmoffatt added a commit that referenced this issue Sep 22, 2020
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.

Co-authored-by: Bryan Moffatt <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant