Skip to content

Commit

Permalink
fix(log): line key does not necessarily exist (#5588)
Browse files Browse the repository at this point in the history
Co-authored-by: Zoltan Kochan <z@kochan.io>
  • Loading branch information
BlackHole1 and zkochan committed Nov 5, 2022
1 parent 2e97907 commit a4c58d4
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 1 deletion.
6 changes: 6 additions & 0 deletions .changeset/lucky-eyes-double.md
@@ -0,0 +1,6 @@
---
"@pnpm/default-reporter": patch
"pnpm": patch
---

The reporter should not crash when the CLI process is kill during lifecycle scripts execution [#5588](https://github.com/pnpm/pnpm/pull/5588).
5 changes: 5 additions & 0 deletions .changeset/moody-weeks-reply.md
@@ -0,0 +1,5 @@
---
"@pnpm/lifecycle": patch
---

Never log a lifecycle exit debug log without an exit code [#5588](https://github.com/pnpm/pnpm/pull/5588).
Expand Up @@ -266,6 +266,7 @@ function formatLine (maxWidth: number, logObj: LifecycleLog) {
}

function cutLine (line: string, maxLength: number) {
if (!line) return '' // This actually should never happen but it is better to be safe
return stripAnsi(line).slice(0, maxLength)
}

Expand Down
46 changes: 46 additions & 0 deletions packages/default-reporter/test/reportingLifecycleScripts.ts
Expand Up @@ -739,6 +739,52 @@ ${STATUS_INDENTATION} ${STATUS_FAILED}`)
})
})

test('do not fail if the debug log has no output', (done) => {
const output$ = toOutput$({
context: { argv: ['install'] },
reportingOptions: { outputMaxWidth: 79 },
streamParser: createStreamParser(),
})

const wd = path.resolve(process.cwd(), 'node_modules', '.registry.npmjs.org', 'foo', '1.0.0', 'node_modules', 'foo')

lifecycleLogger.debug({
depPath: 'registry.npmjs.org/foo/1.0.0',
optional: false,
script: 'node foo',
stage: 'install',
wd,
})
lifecycleLogger.debug({
depPath: 'registry.npmjs.org/foo/1.0.0',
line: undefined as any, // eslint-disable-line @typescript-eslint/no-explicit-any
stage: 'install',
stdio: 'stdout',
wd,
})
lifecycleLogger.debug({
depPath: 'registry.npmjs.org/foo/1.0.0',
exitCode: 1,
optional: false,
stage: 'install',
wd,
})

expect.assertions(1)

output$.pipe(skip(1), take(1), map(normalizeNewline)).subscribe({
complete: () => done(),
error: done,
next: (output: string) => {
expect(replaceTimeWith1Sec(output)).toBe(`\
${chalk.gray('node_modules/.registry.npmjs.org/foo/1.0.0/node_modules/')}foo: Running install script, failed in 1s
.../foo/1.0.0/node_modules/foo ${INSTALL}$ node foo
${OUTPUT_INDENTATION}
${STATUS_INDENTATION} ${STATUS_FAILED}`)
},
})
})

// Many libs use stderr for logging, so showing all stderr adds not much value
test['skip']('prints lifecycle progress', (done) => {
const output$ = toOutput$({
Expand Down
2 changes: 1 addition & 1 deletion packages/lifecycle/src/runLifecycleHook.ts
Expand Up @@ -100,7 +100,7 @@ export async function runLifecycleHook (
// Preventing the pnpm reporter from overriding the project's script output
return
}
const code = arguments[3]
const code = arguments[3] ?? 1
lifecycleLogger.debug({
depPath: opts.depPath,
exitCode: code,
Expand Down

0 comments on commit a4c58d4

Please sign in to comment.