diff --git a/code/lib/cli/src/build.ts b/code/lib/cli/src/build.ts index b92ddd5c37b3..f9b9c2ca1dd1 100644 --- a/code/lib/cli/src/build.ts +++ b/code/lib/cli/src/build.ts @@ -1,11 +1,11 @@ import { sync as readUpSync } from 'read-pkg-up'; import { logger } from '@storybook/node-logger'; -import { buildStaticStandalone } from '@storybook/core-server'; +import { buildStaticStandalone, withTelemetry } from '@storybook/core-server'; import { cache } from '@storybook/core-common'; export const build = async (cliOptions: any) => { try { - await buildStaticStandalone({ + const options = { ...cliOptions, configDir: cliOptions.configDir || './.storybook', outputDir: cliOptions.outputDir || './storybook-static', @@ -14,9 +14,10 @@ export const build = async (cliOptions: any) => { configType: 'PRODUCTION', cache, packageJson: readUpSync({ cwd: __dirname }).packageJson, - }); - } catch (e) { - logger.error(e); + }; + await withTelemetry('build', { presetOptions: options }, () => buildStaticStandalone(options)); + } catch (err) { + logger.error(err); process.exit(1); } }; diff --git a/code/lib/cli/src/dev.ts b/code/lib/cli/src/dev.ts index 922b2f146646..a6aa573c16b8 100644 --- a/code/lib/cli/src/dev.ts +++ b/code/lib/cli/src/dev.ts @@ -1,14 +1,14 @@ import { dedent } from 'ts-dedent'; import { sync as readUpSync } from 'read-pkg-up'; import { logger, instance as npmLog } from '@storybook/node-logger'; -import { buildDevStandalone } from '@storybook/core-server'; +import { buildDevStandalone, withTelemetry } from '@storybook/core-server'; import { cache } from '@storybook/core-common'; export const dev = async (cliOptions: any) => { process.env.NODE_ENV = process.env.NODE_ENV || 'development'; try { - await buildDevStandalone({ + const options = { ...cliOptions, configDir: cliOptions.configDir || './.storybook', configType: 'DEVELOPMENT', @@ -16,7 +16,8 @@ export const dev = async (cliOptions: any) => { docsMode: !!cliOptions.docs, cache, packageJson: readUpSync({ cwd: __dirname }).packageJson, - }); + }; + await withTelemetry('dev', { presetOptions: options }, () => buildDevStandalone(options)); } catch (error) { // this is a weird bugfix, somehow 'node-pre-gyp' is polluting the npmLog header npmLog.heading = ''; diff --git a/code/lib/cli/src/initiate.ts b/code/lib/cli/src/initiate.ts index 30755186ae9b..b0c7825b2a3b 100644 --- a/code/lib/cli/src/initiate.ts +++ b/code/lib/cli/src/initiate.ts @@ -2,6 +2,8 @@ import type { Package } from 'update-notifier'; import chalk from 'chalk'; import prompts from 'prompts'; import { telemetry } from '@storybook/telemetry'; +import { withTelemetry } from '@storybook/core-server'; + import { installableProjectTypes, ProjectType } from './project_types'; import { detect, isStorybookInstalled, detectLanguage, detectBuilder } from './detect'; import { commandLog, codeLog, paddedLog } from './helpers'; @@ -256,7 +258,7 @@ const projectTypeInquirer = async ( return Promise.resolve(); }; -export async function initiate(options: CommandOptions, pkg: Package): Promise { +async function doInitiate(options: CommandOptions, pkg: Package): Promise { const { useNpm, packageManager: pkgMgr } = options; if (useNpm) { useNpmWarning(); @@ -266,7 +268,7 @@ export async function initiate(options: CommandOptions, pkg: Package): Promise { + await withTelemetry('init', { cliOptions: options }, () => doInitiate(options, pkg)); +} diff --git a/code/lib/cli/src/upgrade.ts b/code/lib/cli/src/upgrade.ts index e9dc68079901..57727938e684 100644 --- a/code/lib/cli/src/upgrade.ts +++ b/code/lib/cli/src/upgrade.ts @@ -2,6 +2,8 @@ import { sync as spawnSync } from 'cross-spawn'; import { telemetry } from '@storybook/telemetry'; import semver from 'semver'; import { logger } from '@storybook/node-logger'; +import { withTelemetry } from '@storybook/core-server'; + import { getPackageDetails, JsPackageManagerFactory, @@ -148,7 +150,7 @@ export interface UpgradeOptions { disableTelemetry: boolean; } -export const upgrade = async ({ +export const doUpgrade = async ({ prerelease, skipCheck, useNpm, @@ -188,3 +190,7 @@ export const upgrade = async ({ await automigrate({ dryRun, yes, useNpm, force: pkgMgr }); } }; + +export async function upgrade(options: UpgradeOptions): Promise { + await withTelemetry('upgrade', { cliOptions: options }, () => doUpgrade(options)); +} diff --git a/code/lib/core-server/src/dev-server.ts b/code/lib/core-server/src/dev-server.ts index 89f95bff4b59..90af99a32148 100644 --- a/code/lib/core-server/src/dev-server.ts +++ b/code/lib/core-server/src/dev-server.ts @@ -39,43 +39,33 @@ export async function storybookDevServer(options: Options) { // try get index generator, if failed, send telemetry without storyCount, then rethrow the error let initializedStoryIndexGenerator: Promise = Promise.resolve(undefined); if (features?.buildStoriesJson || features?.storyStoreV7) { - try { - const workingDir = process.cwd(); - const directories = { - configDir: options.configDir, - workingDir, - }; - const normalizedStories = normalizeStories( - await options.presets.apply('stories'), - directories - ); - const storyIndexers = await options.presets.apply('storyIndexers', []); - const docsOptions = await options.presets.apply('docs', {}); - - const generator = new StoryIndexGenerator(normalizedStories, { - ...directories, - storyIndexers, - docs: docsOptions, - workingDir, - storiesV2Compatibility: !features?.breakingChangesV7 && !features?.storyStoreV7, - storyStoreV7: features?.storyStoreV7, - }); + const workingDir = process.cwd(); + const directories = { + configDir: options.configDir, + workingDir, + }; + const normalizedStories = normalizeStories(await options.presets.apply('stories'), directories); + const storyIndexers = await options.presets.apply('storyIndexers', []); + const docsOptions = await options.presets.apply('docs', {}); + + const generator = new StoryIndexGenerator(normalizedStories, { + ...directories, + storyIndexers, + docs: docsOptions, + workingDir, + storiesV2Compatibility: !features?.breakingChangesV7 && !features?.storyStoreV7, + storyStoreV7: features?.storyStoreV7, + }); - initializedStoryIndexGenerator = generator.initialize().then(() => generator); + initializedStoryIndexGenerator = generator.initialize().then(() => generator); - useStoriesJson({ - router, - initializedStoryIndexGenerator, - normalizedStories, - serverChannel, - workingDir, - }); - } catch (err) { - if (!core?.disableTelemetry) { - telemetry('start'); - } - throw err; - } + useStoriesJson({ + router, + initializedStoryIndexGenerator, + normalizedStories, + serverChannel, + workingDir, + }); } if (!core?.disableTelemetry) { @@ -89,7 +79,7 @@ export async function storybookDevServer(options: Options) { }, } : undefined; - telemetry('start', payload, { configDir: options.configDir }); + telemetry('dev', payload, { configDir: options.configDir }); }); } diff --git a/code/lib/core-server/src/index.ts b/code/lib/core-server/src/index.ts index 90f758bdda23..41d3a3b563cc 100644 --- a/code/lib/core-server/src/index.ts +++ b/code/lib/core-server/src/index.ts @@ -9,3 +9,4 @@ export { export * from './build-static'; export * from './build-dev'; +export * from './withTelemetry'; diff --git a/code/lib/core-server/src/withTelemetry.test.ts b/code/lib/core-server/src/withTelemetry.test.ts new file mode 100644 index 000000000000..b60fd8d382f2 --- /dev/null +++ b/code/lib/core-server/src/withTelemetry.test.ts @@ -0,0 +1,215 @@ +/// ; + +import prompts from 'prompts'; +import { loadAllPresets, cache } from '@storybook/core-common'; +import { telemetry } from '@storybook/telemetry'; +import { mocked } from 'ts-jest/utils'; + +import { withTelemetry } from './withTelemetry'; + +jest.mock('prompts'); +jest.mock('@storybook/core-common'); +jest.mock('@storybook/telemetry'); + +it('works in happy path', async () => { + const run = jest.fn(); + + await withTelemetry('dev', {}, run); + + expect(telemetry).toHaveBeenCalledTimes(1); + expect(telemetry).toHaveBeenCalledWith('boot', { eventType: 'dev' }, { stripMetadata: true }); +}); + +describe('when command fails', () => { + const error = new Error('An Error!'); + const run = jest.fn(async () => { + throw error; + }); + + it('sends boot message', async () => { + await expect(async () => withTelemetry('dev', {}, run)).rejects.toThrow(error); + + expect(telemetry).toHaveBeenCalledWith('boot', { eventType: 'dev' }, { stripMetadata: true }); + }); + + it('sends error message when no options are passed', async () => { + await expect(async () => withTelemetry('dev', {}, run)).rejects.toThrow(error); + + expect(telemetry).toHaveBeenCalledTimes(2); + expect(telemetry).toHaveBeenCalledWith( + 'error', + { eventType: 'dev', error }, + expect.objectContaining({}) + ); + }); + + it('does not send error message when cli opt out is passed', async () => { + await expect(async () => + withTelemetry('dev', { cliOptions: { disableTelemetry: true } }, run) + ).rejects.toThrow(error); + + expect(telemetry).toHaveBeenCalledTimes(1); + expect(telemetry).not.toHaveBeenCalledWith( + 'error', + { eventType: 'dev', error }, + expect.objectContaining({}) + ); + }); + + it('does not send error message when crash reports are disabled', async () => { + mocked(loadAllPresets).mockResolvedValueOnce({ + apply: async () => ({ enableCrashReports: false } as any), + }); + await expect(async () => + withTelemetry('dev', { presetOptions: {} as any }, run) + ).rejects.toThrow(error); + + expect(telemetry).toHaveBeenCalledTimes(1); + expect(telemetry).not.toHaveBeenCalledWith( + 'error', + { eventType: 'dev', error }, + expect.objectContaining({}) + ); + }); + + it('does send error message when crash reports are enabled', async () => { + mocked(loadAllPresets).mockResolvedValueOnce({ + apply: async () => ({ enableCrashReports: true } as any), + }); + + await expect(async () => + withTelemetry('dev', { presetOptions: {} as any }, run) + ).rejects.toThrow(error); + + expect(telemetry).toHaveBeenCalledTimes(2); + expect(telemetry).toHaveBeenCalledWith( + 'error', + { eventType: 'dev', error }, + expect.objectContaining({}) + ); + }); + + it('does not send error message when telemetry is disabled', async () => { + mocked(loadAllPresets).mockResolvedValueOnce({ + apply: async () => ({ disableTelemetry: true } as any), + }); + + await expect(async () => + withTelemetry('dev', { presetOptions: {} as any }, run) + ).rejects.toThrow(error); + + expect(telemetry).toHaveBeenCalledTimes(1); + expect(telemetry).not.toHaveBeenCalledWith( + 'error', + expect.objectContaining({}), + expect.objectContaining({}) + ); + }); + + it('does send error messages when telemetry is disabled, but crash reports are enabled', async () => { + mocked(loadAllPresets).mockResolvedValueOnce({ + apply: async () => ({ disableTelemetry: true, enableCrashReports: true } as any), + }); + + await expect(async () => + withTelemetry('dev', { presetOptions: {} as any }, run) + ).rejects.toThrow(error); + + expect(telemetry).toHaveBeenCalledTimes(2); + expect(telemetry).toHaveBeenCalledWith( + 'error', + { eventType: 'dev', error }, + expect.objectContaining({}) + ); + }); + + it('does not send error messages when disabled crash reports are cached', async () => { + mocked(loadAllPresets).mockResolvedValueOnce({ + apply: async () => ({} as any), + }); + mocked(cache.get).mockResolvedValueOnce(false); + + await expect(async () => + withTelemetry('dev', { presetOptions: {} as any }, run) + ).rejects.toThrow(error); + + expect(telemetry).toHaveBeenCalledTimes(1); + expect(telemetry).not.toHaveBeenCalledWith( + 'error', + { eventType: 'dev', error }, + expect.objectContaining({}) + ); + }); + + it('does send error messages when enabled crash reports are cached', async () => { + mocked(loadAllPresets).mockResolvedValueOnce({ + apply: async () => ({} as any), + }); + mocked(cache.get).mockResolvedValueOnce(true); + + await expect(async () => + withTelemetry('dev', { presetOptions: {} as any }, run) + ).rejects.toThrow(error); + + expect(telemetry).toHaveBeenCalledTimes(2); + expect(telemetry).toHaveBeenCalledWith( + 'error', + { eventType: 'dev', error }, + expect.objectContaining({}) + ); + }); + + it('does not send error messages when disabled crash reports are prompted', async () => { + mocked(loadAllPresets).mockResolvedValueOnce({ + apply: async () => ({} as any), + }); + mocked(cache.get).mockResolvedValueOnce(undefined); + mocked(prompts).mockResolvedValueOnce({ enableCrashReports: false }); + + await expect(async () => + withTelemetry('dev', { presetOptions: {} as any }, run) + ).rejects.toThrow(error); + + expect(telemetry).toHaveBeenCalledTimes(1); + expect(telemetry).not.toHaveBeenCalledWith( + 'error', + { eventType: 'dev', error }, + expect.objectContaining({}) + ); + }); + + it('does send error messages when enabled crash reports are prompted', async () => { + mocked(loadAllPresets).mockResolvedValueOnce({ + apply: async () => ({} as any), + }); + mocked(cache.get).mockResolvedValueOnce(undefined); + mocked(prompts).mockResolvedValueOnce({ enableCrashReports: true }); + + await expect(async () => + withTelemetry('dev', { presetOptions: {} as any }, run) + ).rejects.toThrow(error); + + expect(telemetry).toHaveBeenCalledTimes(2); + expect(telemetry).toHaveBeenCalledWith( + 'error', + { eventType: 'dev', error }, + expect.objectContaining({}) + ); + }); + + // if main.js has errors, we have no way to tell if they've disabled telemetry + it('does not send error messages when presets fail to evaluate', async () => { + mocked(loadAllPresets).mockRejectedValueOnce(error); + + await expect(async () => + withTelemetry('dev', { presetOptions: {} as any }, run) + ).rejects.toThrow(error); + + expect(telemetry).toHaveBeenCalledTimes(1); + expect(telemetry).not.toHaveBeenCalledWith( + 'error', + { eventType: 'dev', error }, + expect.objectContaining({}) + ); + }); +}); diff --git a/code/lib/core-server/src/withTelemetry.ts b/code/lib/core-server/src/withTelemetry.ts new file mode 100644 index 000000000000..f147c509defc --- /dev/null +++ b/code/lib/core-server/src/withTelemetry.ts @@ -0,0 +1,85 @@ +import prompts from 'prompts'; +import type { CLIOptions, CoreConfig } from '@storybook/core-common'; +import { loadAllPresets, cache } from '@storybook/core-common'; +import { telemetry } from '@storybook/telemetry'; +import type { EventType } from '@storybook/telemetry'; + +type TelemetryOptions = { + cliOptions?: CLIOptions; + presetOptions?: Parameters[0]; +}; + +const promptCrashReports = async () => { + if (process.env.CI && process.env.NODE_ENV !== 'test') return undefined; + + const { enableCrashReports } = await prompts({ + type: 'confirm', + name: 'enableCrashReports', + message: `Would you like to send crash reports to Storybook?`, + initial: true, + }); + + await cache.set('enableCrashReports', enableCrashReports); + + return enableCrashReports; +}; + +async function shouldSendError({ cliOptions, presetOptions }: TelemetryOptions) { + if (cliOptions?.disableTelemetry) return false; + + // If we are running init or similar, we just have to go with true here + if (!presetOptions) return true; + + // should we load the preset? + const presets = await loadAllPresets({ + corePresets: [require.resolve('./presets/common-preset')], + overridePresets: [], + ...presetOptions, + }); + + // If the user has chosen to enable/disable crash reports in main.js + // or disabled telemetry, we can return that + const core = await presets.apply('core'); + if (core?.enableCrashReports !== undefined) return core.enableCrashReports; + if (core?.disableTelemetry) return false; + + // Deal with typo, remove in future version (7.1?) + const valueFromCache = + (await cache.get('enableCrashReports')) ?? (await cache.get('enableCrashreports')); + if (valueFromCache !== undefined) return valueFromCache; + + const valueFromPrompt = await promptCrashReports(); + if (valueFromPrompt !== undefined) return valueFromPrompt; + + return true; +} + +export async function withTelemetry( + eventType: EventType, + options: TelemetryOptions, + run: () => Promise +) { + telemetry('boot', { eventType }, { stripMetadata: true }); + + try { + await run(); + } catch (error) { + try { + if (await shouldSendError(options)) { + await telemetry( + 'error', + { eventType, error }, + { + immediate: true, + configDir: options.cliOptions?.configDir || options.presetOptions?.configDir, + enableCrashReports: true, + } + ); + } + } catch (err) { + // if this throws an error, we just move on + } + + throw error; + } +} diff --git a/code/lib/telemetry/src/index.ts b/code/lib/telemetry/src/index.ts index 0b883d0ce3d5..f49b77fb7a4d 100644 --- a/code/lib/telemetry/src/index.ts +++ b/code/lib/telemetry/src/index.ts @@ -7,6 +7,8 @@ import { sanitizeError } from './sanitize'; export * from './storybook-metadata'; +export * from './types'; + export const telemetry = async ( eventType: EventType, payload: Payload = {}, @@ -18,23 +20,22 @@ export const telemetry = async ( payload, }; try { - telemetryData.metadata = await getStorybookMetadata(options?.configDir); - } catch (error) { - if (!telemetryData.payload.error) telemetryData.payload.error = error; + if (!options?.stripMetadata) + telemetryData.metadata = await getStorybookMetadata(options?.configDir); + } catch (error: any) { + telemetryData.payload.metadataErrorMessage = sanitizeError(error).message; + if (options?.enableCrashReports) telemetryData.payload.metadataError = sanitizeError(error); } finally { const { error } = telemetryData.payload; - if (error) { - // make sure to anonymise possible paths from error messages - telemetryData.payload.error = sanitizeError(error); - } + // make sure to anonymise possible paths from error messages + if (error) telemetryData.payload.error = sanitizeError(error); if (!telemetryData.payload.error || options?.enableCrashReports) { if (process.env?.STORYBOOK_TELEMETRY_DEBUG) { logger.info('\n[telemetry]'); logger.info(JSON.stringify(telemetryData, null, 2)); - } else { - await sendTelemetry(telemetryData, options); } + await sendTelemetry(telemetryData, options); } } }; diff --git a/code/lib/telemetry/src/sanitize.ts b/code/lib/telemetry/src/sanitize.ts index b2d9f10c8c4b..f3dc5ebde057 100644 --- a/code/lib/telemetry/src/sanitize.ts +++ b/code/lib/telemetry/src/sanitize.ts @@ -32,7 +32,7 @@ export function cleanPaths(str: string, separator: string = sep): string { } // Takes an Error and returns a sanitized JSON String -export function sanitizeError(error: Error, pathSeparator: string = sep): string { +export function sanitizeError(error: Error, pathSeparator: string = sep) { // Hack because Node error = JSON.parse(JSON.stringify(error, Object.getOwnPropertyNames(error))); diff --git a/code/lib/telemetry/src/telemetry.test.ts b/code/lib/telemetry/src/telemetry.test.ts index 66ed9a8ba6a3..24cda35ae8f0 100644 --- a/code/lib/telemetry/src/telemetry.test.ts +++ b/code/lib/telemetry/src/telemetry.test.ts @@ -1,3 +1,5 @@ +/// ; + /* eslint-disable no-plusplus */ import fetch from 'isomorphic-unfetch'; @@ -13,12 +15,12 @@ beforeEach(() => { it('makes a fetch request with name and data', async () => { fetchMock.mockClear(); - await sendTelemetry({ eventType: 'start', payload: { foo: 'bar' } }); + await sendTelemetry({ eventType: 'dev', payload: { foo: 'bar' } }); expect(fetch).toHaveBeenCalledTimes(1); const body = JSON.parse(fetchMock.mock.calls[0][1].body); expect(body).toMatchObject({ - eventType: 'start', + eventType: 'dev', payload: { foo: 'bar' }, }); }); @@ -27,7 +29,7 @@ it('retries if fetch fails with a 503', async () => { fetchMock.mockClear().mockResolvedValueOnce({ status: 503 }); await sendTelemetry( { - eventType: 'start', + eventType: 'dev', payload: { foo: 'bar' }, }, { retryDelay: 0 } @@ -40,7 +42,7 @@ it('gives up if fetch repeatedly fails', async () => { fetchMock.mockClear().mockResolvedValue({ status: 503 }); await sendTelemetry( { - eventType: 'start', + eventType: 'dev', payload: { foo: 'bar' }, }, { retryDelay: 0 } @@ -63,7 +65,7 @@ it('await all pending telemetry when passing in immediate = true', async () => { numberOfResolvedTasks++; }); sendTelemetry({ - eventType: 'start', + eventType: 'dev', payload: { foo: 'bar' }, }).then(() => { numberOfResolvedTasks++; @@ -72,7 +74,7 @@ it('await all pending telemetry when passing in immediate = true', async () => { // here we await await sendTelemetry( { - eventType: 'error-dev', + eventType: 'error', payload: { foo: 'bar' }, }, { retryDelay: 0, immediate: true } diff --git a/code/lib/telemetry/src/telemetry.ts b/code/lib/telemetry/src/telemetry.ts index 2ecab470b4dd..40ea66c91691 100644 --- a/code/lib/telemetry/src/telemetry.ts +++ b/code/lib/telemetry/src/telemetry.ts @@ -25,10 +25,12 @@ export async function sendTelemetry( // flatten the data before we send it const { payload, metadata, ...rest } = data; - const context = { - anonymousId: getAnonymousProjectId(), - inCI: process.env.CI === 'true', - }; + const context = options.stripMetadata + ? {} + : { + anonymousId: getAnonymousProjectId(), + inCI: process.env.CI === 'true', + }; const eventId = nanoid(); const body = { ...rest, eventId, sessionId, metadata, payload, context }; let request: Promise; diff --git a/code/lib/telemetry/src/types.ts b/code/lib/telemetry/src/types.ts index 3014d3fa1f15..b251fae45ca6 100644 --- a/code/lib/telemetry/src/types.ts +++ b/code/lib/telemetry/src/types.ts @@ -3,14 +3,7 @@ import type { PM } from 'detect-package-manager'; import type { MonorepoType } from './get-monorepo-type'; -export type EventType = - | 'start' - | 'build' - | 'upgrade' - | 'init' - | 'error-build' - | 'error-dev' - | 'error-metadata'; +export type EventType = 'boot' | 'dev' | 'build' | 'upgrade' | 'init' | 'error' | 'error-metadata'; export interface Dependency { version: string | undefined; @@ -62,6 +55,7 @@ export interface Options { immediate: boolean; configDir?: string; enableCrashReports?: boolean; + stripMetadata?: boolean; } export interface TelemetryData {