-
Notifications
You must be signed in to change notification settings - Fork 6k
Fix marketplace in CLI #4301
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
Fix marketplace in CLI #4301
Conversation
- Fix error logging from child process.
✨ Coder.com for PR #4301 deployed! It will be updated on every commit.
|
@@ -22,8 +22,8 @@ export const runVsCodeCli = async (args: DefaultedArgs): Promise<void> => { | |||
|
|||
try { | |||
await cliProcessMain(args) | |||
} catch (error) { | |||
logger.error("Got error from VS Code", field("error", error)) | |||
} catch (error: any) { |
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.
just curious, are there root error types in Node or TypeScript that we can use instead of any
? seems unfortunate not to have any type information
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 think it's traditionally been any
unfortunately. But in TypeScript 4.0, they allow you to type it as unknown
which they say is safer. But yeah, it's too bad they don't tell us which error this is.
@@ -22,8 +22,8 @@ export const runVsCodeCli = async (args: DefaultedArgs): Promise<void> => { | |||
|
|||
try { | |||
await cliProcessMain(args) | |||
} catch (error) { | |||
logger.error("Got error from VS Code", field("error", error)) | |||
} catch (error: any) { |
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 think it's traditionally been any
unfortunately. But in TypeScript 4.0, they allow you to type it as unknown
which they say is safer. But yeah, it's too bad they don't tell us which error this is.
This PR resolves an issue where our marketplace modifications are not applied to the CLI.