Skip to content

Commit

Permalink
rework Zinnia errors reported to Sentry (#378)
Browse files Browse the repository at this point in the history
- Add the module name & the exit code to the message
- Remove Zinnia & module main file path from the message

The goal is to allow Sentry to group all errors with the same cause
in a single issue.

Add the last 50 lines from the Execa error message (including 
stdout & stderr) into the `extra` fields sent to Sentry.

Signed-off-by: Miroslav Bajtoš <[email protected]>
Co-authored-by: Julian Gruber <[email protected]>
  • Loading branch information
bajtos and juliangruber authored Mar 20, 2024
1 parent 55bd915 commit 520fe2c
Showing 1 changed file with 27 additions and 10 deletions.
37 changes: 27 additions & 10 deletions lib/zinnia.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,14 @@ const maybeReportErrorToSentry = (/** @type {unknown} */ err) => {

/** @type {Parameters<Sentry.captureException>[1]} */
const hint = {}
// Detect execa errors, see
// https://github.com/sindresorhus/execa?tab=readme-ov-file#execasyncerror
if (err instanceof Error && 'shortMessage' in err && 'originalMessage' in err) {
if (typeof err === 'object' && 'details' in err && typeof err.details === 'string') {
// Quoting from https://develop.sentry.dev/sdk/data-handling/
// > Messages are limited to 8192 characters.
// > Individual extra data items are limited to 16kB. Total extra data is limited to 256kb.
// Let's store the full message including stdout && stderr in an extra field
// Let's store the additional details (e.g. stdout && stderr) in an extra field
const tail = err.details.split(/\n/g).slice(-50).join('\n')
hint.extra = {
details: err.message
details: tail
}
}

Expand Down Expand Up @@ -210,11 +209,29 @@ const catchChildProcessExit = async ({
onActivity
}) => {
try {
await Promise.race(childProcesses)
const tasks = childProcesses.map(p => (async () => {
try {
await p
} catch (err) {
// When the child process crash, attach the module name & the exit reason to the error object
const exitReason = p.exitCode
? `with exit code ${p.exitCode}`
: p.signalCode ? `via signal ${p.signalCode}` : undefined
throw Object.assign(err, { moduleName: p.moduleName, exitReason })
}
})())

await Promise.race(tasks)
} catch (err) {
if (!shouldRestart.get()) {
onActivity({ type: 'error', message: 'Zinnia crashed' })
maybeReportErrorToSentry(err)
const moduleName = capitalize(err.moduleName ?? 'Zinnia')
const exitReason = err.exitReason ?? 'for unknown reason'
const message = `${moduleName} crashed ${exitReason}`
onActivity({ type: 'error', message })
const moduleErr = new Error(message, { cause: err })
// Store the full error message including stdout & stder in the top-level `details` property
Object.assign(moduleErr, { details: err.message })
maybeReportErrorToSentry(moduleErr)
throw err
}
} finally {
Expand All @@ -228,7 +245,7 @@ const waitForStdioClose = async ({ childProcesses, controller }) => {
const results = await Promise.all(
childProcesses.map(childProcess => once(childProcess, 'close'))
)
const codes = results.map(([code]) => code ?? '<no code>')
const codes = results.map(([code, signal]) => code || `<${signal}>` || '<no code>')
console.error(`Zinnia closed all stdio with codes ${codes.join(', ')}`)
for (const childProcess of childProcesses) {
childProcess.stderr.removeAllListeners()
Expand Down Expand Up @@ -308,7 +325,7 @@ export async function run ({
}
}
)
childProcesses.push(childProcess)
childProcesses.push(Object.assign(childProcess, { moduleName: module }))

childProcess.stdout.setEncoding('utf-8')
childProcess.stdout.on('data', data => {
Expand Down

0 comments on commit 520fe2c

Please sign in to comment.