-
Notifications
You must be signed in to change notification settings - Fork 97
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
Improve function invocation error #377
Conversation
@@ -137,6 +137,10 @@ func (f *Function) callAWSLambda(payload []byte) ([]byte, error) { | |||
if err != nil { | |||
if awserr, ok := err.(awserr.Error); ok { | |||
switch awserr.Code() { | |||
case "AccessDeniedException": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@elsteelbrain I did it differently, please take a look
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tested it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, of course.
function/function.go
Outdated
return nil, &ErrFunctionError{err} | ||
} | ||
awslambda := lambda.New(awsSession) | ||
awslambda := lambda.New(session.New(config)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is deprecated!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I will revert it
function/function.go
Outdated
|
||
invokeOutput, err := awslambda.Invoke(&lambda.InvokeInput{ | ||
FunctionName: &f.Provider.ARN, | ||
Payload: payload, | ||
}) | ||
if err != nil { | ||
if receivedAWSErr, ok := err.(awserr.Error); ok { | ||
switch receivedAWSErr.Code() { | ||
if awserr, ok := err.(awserr.Error); ok { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awserr
collides with an imported package
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there is no automated check for that (with gometalinter
) I would assume it's fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My linter is reporting a warning on this. Not that it would be problematic, just that it is bad practice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are many places like that in the codebase. We can fix it but in separate PR, and it has to be automatically checked by gometalinter
. AFAIR there is a check for that in gometalinter
. Because of some reason, it has been turned off.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
No description provided.