fix: prevent reporting expected module restart errors to Sentry (#599)

lib/zinnia.js

@@ -1,5 +1,5 @@
 import { execa } from 'execa'
-import * as Sentry from '@sentry/node'
+import { withScope, captureException } from '@sentry/node' //imported here to support fingerprint grouping in Sentry
 import { installRuntime, getRuntimeExecutable } from './runtime.js'
 import { updateSourceFiles } from './subnets.js'
 import os from 'node:os'
@@ -53,18 +53,37 @@ export const install = () =>
   })
 
 let lastErrorReportedAt = 0
-const maybeReportErrorToSentry = (/** @type {unknown} */ err) => {
+/** @param {unknown} err */
+function isErrorWithSentryFlag(err) {
+  return typeof err === 'object' && err !== null && 'reportToSentry' in err
+}
+
+/**
+ * Reports errors to Sentry, unless reportToSentry is explicitly false. Uses
+ * dependency injection to allow clean unit testing.
+ *
+ * @param {unknown} err
+ * @param {(error: unknown, hint?: object) => void} captureFn - Optional Sentry
+ *   capture function (default: captureException)
+ */
+export function maybeReportErrorToSentry(err, captureFn = captureException) {
+  if (
+    isErrorWithSentryFlag(err) &&
+    /** @type {{ reportToSentry?: boolean }} */ (err).reportToSentry === false
+  ) {
+    return
+  }
+
   const now = Date.now()
   if (now - lastErrorReportedAt < 4 /* HOURS */ * 3600_000) return
   lastErrorReportedAt = now
 
-  /** @type {Parameters<Sentry.captureException>[1]} */
+  /** @type {Parameters<typeof captureFn>[1]} */
   const hint = { extra: {} }
-  if (typeof err === 'object') {
-    if ('reportToSentry' in err && err.reportToSentry === false) {
-      return
-    }
+  if (typeof err === 'object' && err !== null) {
+    // Mark error to prevent future duplicate reporting
     Object.assign(err, { reportToSentry: false })
+
     if ('details' in err && typeof err.details === 'string') {
       // Quoting from https://develop.sentry.dev/sdk/data-handling/
       // > Messages are limited to 8192 characters.
@@ -81,7 +100,7 @@ const maybeReportErrorToSentry = (/** @type {unknown} */ err) => {
   console.error(
     'Reporting the problem to Sentry for inspection by the Checker team.',
   )
-  Sentry.captureException(err, hint)
+  captureFn(err, hint)
 }
 
 const matchesSubnetFilter = (subnet) =>
@@ -232,7 +251,7 @@ const catchChildProcessExit = async ({
       } else {
         // Apply a custom rule to force Sentry to group all issues with the same subnet & exit code
         // See https://docs.sentry.io/platforms/node/usage/sdk-fingerprinting/#basic-example
-        Sentry.withScope((scope) => {
+        withScope((scope) => {
           scope.setFingerprint([message])
           maybeReportErrorToSentry(subnetErr)
         })
@@ -319,7 +338,7 @@ export async function run({
           })
 
           const err = new Error('Module inactive for 5 minutes')
-          Object.assign(err, { module })
+          Object.assign(err, { module, reportToSentry: false })
           maybeReportErrorToSentry(err)
 
           controller.abort()
@@ -340,7 +359,7 @@ export async function run({
         text: data,
       }).catch((err) => {
         console.error(err)
-        Sentry.captureException(err)
+        maybeReportErrorToSentry(err)
       })
     })
     childProcess.stderr.setEncoding('utf-8')

package.json

@@ -64,6 +64,7 @@
     "prettier-plugin-jsdoc": "^1.3.2",
     "prettier-plugin-multiline-arrays": "^4.0.3",
     "prettier-plugin-packagejson": "^2.5.10",
+    "sinon": "^20.0.0",
     "stream-match": "^1.2.1",
     "typescript": "^5.0.4"
   },

test/zinnia.test.js

@@ -0,0 +1,25 @@
+// Unit test for maybeReportErrorToSentry using dependency-injected spy.
+// This avoids mocking ESM modules directly.
+import assert from 'node:assert'
+import sinon from 'sinon'
+import { maybeReportErrorToSentry } from '../lib/zinnia.js'
+
+describe('maybeReportErrorToSentry', () => {
+  it('should NOT report error when reportToSentry is false', () => {
+    // Inject spy instead of using real Sentry
+    const spy = sinon.spy()
+    const error = new Error('Expected error')
+    error.reportToSentry = false
+
+    maybeReportErrorToSentry(error, spy)
+    assert.strictEqual(spy.called, false)
+  })
+
+  it('should report error when reportToSentry is not set', () => {
+    const spy = sinon.spy()
+    const error = new Error('Unexpected error')
+
+    maybeReportErrorToSentry(error, spy)
+    assert.strictEqual(spy.called, true)
+  })
+})