Small fixes in telemetry and plugin init #113
Conversation
| log.debug(`project: ${projectName}`) | ||
| const envId = flags.id ?? await findAmbientEnvId(projectName) | ||
|
|
||
| const [envId, userModel] = flags.id ? [flags.id, { name: flags.id }] : await (async () => { |
There was a problem hiding this comment.
envId is incorrect here - missing branch suffix.
what is this fixing?
There was a problem hiding this comment.
The case of using down --id without docker/compose. (#114)
There was a problem hiding this comment.
if the project name is only needed for the debug.log call, I suggest removing it to simplify:
| const [envId, userModel] = flags.id ? [flags.id, { name: flags.id }] : await (async () => { | |
| const envId = flags.id ?? await findAmbientEnvId((await this.ensureUserModel()).name) |
There was a problem hiding this comment.
That was the variable before.
const envId = flags.id ?? await findAmbientEnvId(projectName)
|
|
||
| const wrappedHook: OclifHook<'init'> = async function wrappedHook(...args) { | ||
| try { | ||
| const { id } = args[0] |
There was a problem hiding this comment.
IMO this function shouldn't include any logic besides wrapping with try..catch, you can move the id check to the hook itself.
| @@ -71,7 +72,7 @@ export const childProcessStdoutPromise = async ( | |||
| return orderedOutput(output).stdout().toString('utf-8').trim() | |||
| } | |||
|
|
|||
| export const spawnPromise = ( | |||
| export const spawnPromise = async ( | |||
There was a problem hiding this comment.
The async isn't redundant, if spawn logic throw the error is not wrapped.
Ideally, any function that return a Promise should give a strong guarantee that the exception is always wrapped in the promise return value.
async force that, for example:
await ((function(){ throw "abc"; return Promise.resolve(5)}()).catch(e=> { }))
throws "abc"
await ((async function(){ throw "abc"; return Promise.resolve(5)}()).catch(e=> { }))
doesn't throw
|
Went with Remote no-return-await (to avoid synchronous throws and use async stack-traces: https://v8.dev/blog/fast-async#improved-developer-experience) and force typescript to require the use of |
In addition, this fix a bug of unhandled exception and additional telemetry