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 nodejs automation api inline error handling #6237
Conversation
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.
Couple of questions but otherwise LGTM!
const defaultMessage = err.stack || err.message || ("" + err); | ||
let defaultMessage = "" | ||
if (!!err) { | ||
defaultMessage = err.stack || err.message || ("" + err); |
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.
What's the relevance of the "" + err
? Is that a string conversion?
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.
yup
sdk/nodejs/x/automation/stack.ts
Outdated
let upResult; | ||
try { | ||
upResult = await this.runPulumiCmd(args, opts?.onOutput); | ||
onExit(); |
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.
Is this onExit()
necessary? Doesn't the finally
block run regardless of 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.
You are right. Updated.
This changeset adds several improvements to our error handling for nodejs inline programs run via Automation API. Along the way, I also cleaned up a bunch of unused functionality from
server.ts
(exposing aresult
promise, providing an unused CLI exit code, etc). Summary of fixes:LanguageServer
is always torn down cleanly (otherwise causes failed programs to hang)Error
are handled properly, even if user code throws an error without any details, exception message, etc.LanguageServer.Run
should respond with errors via calling thecallback
function https://avi.im/grpc-errors/#nodeFixes #6153
Fixes #5853
Fixes #6057