chore(cli-storybook): Add telemetry

packages/cli-packages/storybook/package.json

@@ -24,6 +24,7 @@
     ]
   },
   "dependencies": {
+    "@redwoodjs/cli-helpers": "5.0.0",
     "@redwoodjs/project-config": "5.0.0",
     "@redwoodjs/telemetry": "5.0.0",
     "@storybook/addon-a11y": "7.0.27",

packages/cli-packages/storybook/src/commands/storybook.ts

@@ -1,7 +1,8 @@
 import terminalLink from 'terminal-link'
 import type { Argv } from 'yargs'
 
-import c from '../lib/colors'
+import { recordTelemetryAttributes } from '@redwoodjs/cli-helpers'
+
 import { StorybookYargsOptions } from '../types'
 
 export const command = 'storybook'
@@ -54,22 +55,6 @@ export function builder(
       default: defaultOptions.smokeTest,
     })
 
-    .check((argv) => {
-      if (argv.build && argv.smokeTest) {
-        throw new Error('Can not provide both "--build" and "--smoke-test"')
-      }
-
-      if (argv.build && argv.open) {
-        console.warn(
-          c.warning(
-            'Warning: --open option has no effect when running Storybook build'
-          )
-        )
-      }
-
-      return true
-    })
-
     .epilogue(
       `Also see the ${terminalLink(
         'Redwood CLI Reference',
@@ -82,6 +67,14 @@ export async function handler(options: StorybookYargsOptions): Promise<void> {
   // NOTE: We should provide some visual output before the import to increase
   // the perceived performance of the command as there will be delay while we
   // load the handler.
+  recordTelemetryAttributes({
+    command: 'storybook',
+    build: options.build,
+    ci: options.ci,
+    open: options.open,
+    smokeTest: options.smokeTest,
+  })
+
   const { handler: storybookHandler } = await import('./storybookHandler.js')
   await storybookHandler(options)
 }

packages/cli-packages/storybook/src/commands/storybookHandler.ts

@@ -1,6 +1,6 @@
 import path from 'node:path'
 
-import execa from 'execa'
+import execa, { ExecaError } from 'execa'
 
 import { getPaths } from '@redwoodjs/project-config'
 // Allow import of untyped package
@@ -19,6 +19,19 @@ export async function handler({
   port,
   smokeTest,
 }: StorybookYargsOptions) {
+  // Check for conflicting options
+  if (build && smokeTest) {
+    throw new Error('Can not provide both "--build" and "--smoke-test"')
+  }
+
+  if (build && open) {
+    console.warn(
+      c.warning(
+        'Warning: --open option has no effect when running Storybook build'
+      )
+    )
+  }
+
   const cwd = getPaths().web.base
   const staticAssetsFolder = path.join(cwd, 'public')
   const execaOptions: Partial<execa.Options> = {
@@ -72,8 +85,10 @@ export async function handler({
   try {
     await execa.command(command, execaOptions)
   } catch (e) {
-    console.log(c.error((e as Error).message))
-    errorTelemetry(process.argv, (e as Error).message)
-    process.exit(1)
+    if ((e as ExecaError).signal !== 'SIGINT') {
+      console.log(c.error((e as Error).message))
+      errorTelemetry(process.argv, (e as Error).message)
+    }
+    process.exit((e as ExecaError).exitCode ?? 1)
   }
 }

packages/cli/src/telemetry/index.js

@@ -66,7 +66,23 @@ export async function startTelemetry() {
     traceProvider.addSpanProcessor(traceProcessor)
     traceProvider.register()
 
-    // Ensure to shutdown telemetry when the process exits
+    // Without any listeners for these signals, nodejs will terminate the process and will not
+    // trigger the exit event when doing so. This means our process.on('exit') handler will not run.
+    // We add a listner which either calls process.exit or if some other handler has been added,
+    // 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()
+        }
+      })
+    }
+
+    // Ensure to shutdown telemetry when the process exits so that we can be sure that all spans
+    // are ended and all data is flushed to the exporter.
     process.on('exit', () => {
       shutdownTelemetry()
     })

yarn.lock

@@ -7591,6 +7591,7 @@ __metadata:
   version: 0.0.0-use.local
   resolution: "@redwoodjs/cli-storybook@workspace:packages/cli-packages/storybook"
   dependencies:
+    "@redwoodjs/cli-helpers": 5.0.0
     "@redwoodjs/project-config": 5.0.0
     "@redwoodjs/telemetry": 5.0.0
     "@storybook/addon-a11y": 7.0.27