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
Modern logs for invoke local
command
#10065
Conversation
Codecov Report
@@ Coverage Diff @@
## master #10065 +/- ##
==========================================
- Coverage 85.53% 85.50% -0.03%
==========================================
Files 333 333
Lines 13473 13528 +55
==========================================
+ Hits 11524 11567 +43
- Misses 1949 1961 +12
Continue to review full report at Codecov.
|
lib/plugins/aws/invokeLocal/index.js
Outdated
@@ -773,7 +819,8 @@ class AwsInvokeLocal { | |||
const handlersContainer = require(pathToHandler); | |||
lambda = handlersContainer[handlerName]; | |||
} catch (error) { | |||
this.serverless.cli.consoleLog(chalk.red(inspect(error))); | |||
legacy.consoleLog(chalk.red(inspect(error))); | |||
writeText(inspect(error)); |
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.
Shouldn't it be log.error
?
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.
I don't think so - it's an error that was thrown by invoked Lambda function - I think we should treat it as an output of the command because that's what it really is. What do you think?
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.
Nevermind, I though it's a different part of the code - you're right, it should be an error here
2a914bb
to
bdea74e
Compare
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.
Looks great to me, still there's test fail on Windows
bdea74e
to
0c787d8
Compare
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.
Well done @pgrzesik !
This tests refactor was definitely painful part, great you've addressed that
Addresses: #9860