From 2142ca5431458e30cfe138072bbc6460554ea3be Mon Sep 17 00:00:00 2001
From: Josh GM Walker <56300765+Josh-Walker-GM@users.noreply.github.com>
Date: Thu, 27 Jul 2023 22:57:39 +0100
Subject: [PATCH] fix(cli): Prevent unwanted SIGINT error message from
 storybook (#8977)

**Problem**
Storybook is showing an error message when you exit via a SIGINT.

We have spent a few hours trying to understand why and work around this
with no success.

*Changes**
1. Conditionally disable the additional default signal listeners based
on the argv contents.
2. Emit a custom event on `process` to trigger the telemetry shutdown.

**Notes**
I'm not really proud of this PR. Disabling based on the contents of argv
isn't all that robust against false positives. Emitting a custom event
on `process` is not really supposed to be done - hence why TS is
complaining about the name of the signal.

This work has also highlighted that on my windows 11 machine the
background scripts are not firing... This was working when I updated the
span values to match those found to work in this PR comment
https://github.com/nodejs/node/issues/21825#issuecomment-503766781 when
I reenabled windows telemetry. Maybe something has changed, maybe it's
version specific (os or node). This is an active nodejs issue so my
ability to solve this in redwood code is understandably limited.

Why isn't our CI finding this? We should investigate this too.

@jtoar Could you confirm on mac and windows that this PR prevents those
additional error messages from storybook. If you also have the time I'd
appreciate if you could check if the telemetry background job is firing
when you test on windows.

I'll test this on ubuntu locally.
---
 .../storybook/src/commands/storybook.ts       |  2 ++
 packages/cli/src/telemetry/index.js           | 20 +++++++++++++------
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/packages/cli-packages/storybook/src/commands/storybook.ts b/packages/cli-packages/storybook/src/commands/storybook.ts
index 0992905bf73a..1c8afb22582b 100644
--- a/packages/cli-packages/storybook/src/commands/storybook.ts
+++ b/packages/cli-packages/storybook/src/commands/storybook.ts
@@ -74,6 +74,8 @@ export async function handler(options: StorybookYargsOptions): Promise<void> {
     open: options.open,
     smokeTest: options.smokeTest,
   })
+  // @ts-expect-error - Custom workaround for storybook telemetry
+  process.emit('shutdown-telemetry')
 
   const { handler: storybookHandler } = await import('./storybookHandler.js')
   await storybookHandler(options)
diff --git a/packages/cli/src/telemetry/index.js b/packages/cli/src/telemetry/index.js
index 2eb18672c169..b8160d21e0a5 100644
--- a/packages/cli/src/telemetry/index.js
+++ b/packages/cli/src/telemetry/index.js
@@ -7,6 +7,7 @@ import {
   SimpleSpanProcessor,
   SamplingDecision,
 } from '@opentelemetry/sdk-trace-node'
+import { hideBin } from 'yargs/helpers'
 
 import { spawnBackgroundProcess } from '../lib/background'
 
@@ -72,12 +73,19 @@ export async function startTelemetry() {
     // then we leave it to that handler to handle the signal.
     // See https://nodejs.org/dist/latest/docs/api/process.html#signal-events for more info on the
     // behaviour of nodejs for various signals.
-    for (const signal of ['SIGTERM', 'SIGINT', 'SIGHUP']) {
-      process.on(signal, () => {
-        if (process.listenerCount(signal) === 1) {
-          console.log(`Received ${signal} signal, exiting...`)
-          process.exit()
-        }
+    const cleanArgv = hideBin(process.argv)
+    if (!cleanArgv.includes('sb') && !cleanArgv.includes('storybook')) {
+      for (const signal of ['SIGTERM', 'SIGINT', 'SIGHUP']) {
+        process.on(signal, () => {
+          if (process.listenerCount(signal) === 1) {
+            console.log(`Received ${signal} signal, exiting...`)
+            process.exit()
+          }
+        })
+      }
+    } else {
+      process.on('shutdown-telemetry', () => {
+        shutdownTelemetry()
       })
     }