diff --git a/code/core/src/core-server/withTelemetry.test.ts b/code/core/src/core-server/withTelemetry.test.ts index a46488bbe561..af02bfac6936 100644 --- a/code/core/src/core-server/withTelemetry.test.ts +++ b/code/core/src/core-server/withTelemetry.test.ts @@ -1,6 +1,6 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { cache, loadAllPresets } from 'storybook/internal/common'; +import { cache, isCI, loadAllPresets } from 'storybook/internal/common'; import { prompt } from 'storybook/internal/node-logger'; import { ErrorCollector, oneWayHash, telemetry } from 'storybook/internal/telemetry'; @@ -11,6 +11,18 @@ vi.mock('storybook/internal/telemetry', { spy: true }); vi.mock('storybook/internal/node-logger', { spy: true }); const cliOptions = {}; +const originalStdoutIsTTY = process.stdout.isTTY; + +const setStdoutIsTTY = (value: boolean | undefined) => { + Object.defineProperty(process.stdout, 'isTTY', { + value, + configurable: true, + }); +}; + +afterEach(() => { + setStdoutIsTTY(originalStdoutIsTTY); +}); describe('withTelemetry', () => { beforeEach(() => { @@ -74,6 +86,52 @@ describe('withTelemetry', () => { ); }); + it('prompts for crash reports when init fails without preset options', async () => { + vi.mocked(isCI).mockReturnValue(false); + vi.mocked(cache.get).mockResolvedValueOnce(undefined); + vi.mocked(prompt.confirm).mockResolvedValueOnce(true); + setStdoutIsTTY(true); + + await expect(async () => + withTelemetry('init', { cliOptions, printError: vi.fn() }, run) + ).rejects.toThrow(error); + + expect(prompt.confirm).toHaveBeenCalledTimes(1); + expect(cache.set).toHaveBeenCalledWith('enableCrashReports', true); + expect(telemetry).toHaveBeenCalledWith( + 'error', + expect.objectContaining({ + eventType: 'init', + error: expect.objectContaining({ message: 'An Error!', name: 'Error' }), + isErrorInstance: true, + }), + expect.objectContaining({ enableCrashReports: true }) + ); + }); + + it('does not send full error details when init prompt is rejected', async () => { + vi.mocked(isCI).mockReturnValue(false); + vi.mocked(cache.get).mockResolvedValueOnce(undefined); + vi.mocked(prompt.confirm).mockResolvedValueOnce(false); + setStdoutIsTTY(true); + + await expect(async () => + withTelemetry('init', { cliOptions, printError: vi.fn() }, run) + ).rejects.toThrow(error); + + expect(prompt.confirm).toHaveBeenCalledTimes(1); + expect(cache.set).toHaveBeenCalledWith('enableCrashReports', false); + expect(telemetry).toHaveBeenCalledWith( + 'error', + expect.objectContaining({ + eventType: 'init', + error: undefined, + isErrorInstance: true, + }), + expect.objectContaining({ enableCrashReports: false }) + ); + }); + it('does not send error message when cli opt out is passed', async () => { await expect(async () => withTelemetry('dev', { cliOptions: { disableTelemetry: true }, printError: vi.fn() }, run) @@ -383,6 +441,67 @@ describe('sendTelemetryError', () => { }) ); }); + + it('does not prompt for non-blocking init errors without cached consent', async () => { + const options: any = { + cliOptions: {}, + skipPrompt: false, + }; + const mockError = new Error('Init non-blocking error'); + + vi.mocked(isCI).mockReturnValue(false); + vi.mocked(cache.get).mockResolvedValueOnce(undefined); + vi.mocked(prompt.confirm).mockResolvedValueOnce(true); + setStdoutIsTTY(true); + + await sendTelemetryError(mockError, 'init', options, false); + + expect(prompt.confirm).not.toHaveBeenCalled(); + expect(vi.mocked(cache.set).mock.calls).not.toContainEqual([ + 'enableCrashReports', + expect.anything(), + ]); + expect(telemetry).toHaveBeenCalledWith( + 'error', + expect.objectContaining({ + eventType: 'init', + blocking: false, + error: undefined, + isErrorInstance: true, + }), + expect.objectContaining({ + enableCrashReports: false, + immediate: true, + }) + ); + }); + + it('uses cached crash report consent for non-blocking init errors', async () => { + const options: any = { + cliOptions: {}, + skipPrompt: false, + }; + const mockError = new Error('Init non-blocking error'); + + vi.mocked(cache.get).mockResolvedValueOnce(true); + + await sendTelemetryError(mockError, 'init', options, false); + + expect(prompt.confirm).not.toHaveBeenCalled(); + expect(telemetry).toHaveBeenCalledWith( + 'error', + expect.objectContaining({ + eventType: 'init', + blocking: false, + error: expect.objectContaining({ message: 'Init non-blocking error', name: 'Error' }), + isErrorInstance: true, + }), + expect.objectContaining({ + enableCrashReports: true, + immediate: true, + }) + ); + }); }); describe('getErrorLevel', () => { @@ -412,6 +531,7 @@ describe('getErrorLevel', () => { }, presetOptions: undefined, skipPrompt: false, + eventType: 'dev', }; const errorLevel = await getErrorLevel(options); @@ -419,6 +539,52 @@ describe('getErrorLevel', () => { expect(errorLevel).toBe('error'); }); + it('returns "full" for init when presetOptions are not provided and prompt is accepted', async () => { + const options: any = { + cliOptions: { + disableTelemetry: false, + }, + presetOptions: undefined, + skipPrompt: false, + eventType: 'init', + }; + + vi.mocked(isCI).mockReturnValue(false); + vi.mocked(cache.get).mockResolvedValueOnce(undefined); + vi.mocked(prompt.confirm).mockResolvedValueOnce(true); + setStdoutIsTTY(true); + + const errorLevel = await getErrorLevel(options); + + expect(errorLevel).toBe('full'); + expect(loadAllPresets).not.toHaveBeenCalled(); + expect(prompt.confirm).toHaveBeenCalledTimes(1); + expect(cache.set).toHaveBeenCalledWith('enableCrashReports', true); + }); + + it('returns "error" for init when presetOptions are not provided and prompt is rejected', async () => { + const options: any = { + cliOptions: { + disableTelemetry: false, + }, + presetOptions: undefined, + skipPrompt: false, + eventType: 'init', + }; + + vi.mocked(isCI).mockReturnValue(false); + vi.mocked(cache.get).mockResolvedValueOnce(undefined); + vi.mocked(prompt.confirm).mockResolvedValueOnce(false); + setStdoutIsTTY(true); + + const errorLevel = await getErrorLevel(options); + + expect(errorLevel).toBe('error'); + expect(loadAllPresets).not.toHaveBeenCalled(); + expect(prompt.confirm).toHaveBeenCalledTimes(1); + expect(cache.set).toHaveBeenCalledWith('enableCrashReports', false); + }); + it('returns "full" when core.enableCrashReports is true', async () => { const options: any = { cliOptions: { diff --git a/code/core/src/core-server/withTelemetry.ts b/code/core/src/core-server/withTelemetry.ts index a7e77bd42e83..a8eb4738fcbf 100644 --- a/code/core/src/core-server/withTelemetry.ts +++ b/code/core/src/core-server/withTelemetry.ts @@ -16,6 +16,7 @@ type TelemetryOptions = { presetOptions?: Parameters[0]; printError?: (err: any) => void; skipPrompt?: boolean; + eventType?: EventType; }; const promptCrashReports = async () => { @@ -40,29 +41,30 @@ export async function getErrorLevel({ cliOptions, presetOptions, skipPrompt, + eventType, }: TelemetryOptions): Promise { if (cliOptions.disableTelemetry) { return 'none'; } - // If we are running init or similar, we just have to go with true here - if (!presetOptions) { + if (!presetOptions && eventType !== 'init') { return 'error'; } - // should we load the preset? - const presets = await loadAllPresets(presetOptions); + if (presetOptions) { + const presets = await loadAllPresets(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 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 ? 'full' : 'error'; - } + if (core?.enableCrashReports !== undefined) { + return core.enableCrashReports ? 'full' : 'error'; + } - if (core?.disableTelemetry) { - return 'none'; + if (core?.disableTelemetry) { + return 'none'; + } } // Deal with typo, remove in future version (7.1?) @@ -96,7 +98,11 @@ export async function sendTelemetryError( try { let errorLevel = 'error'; try { - errorLevel = await getErrorLevel(options); + errorLevel = await getErrorLevel({ + ...options, + eventType, + skipPrompt: options.skipPrompt || (eventType === 'init' && !blocking), + }); } catch (err) { // If this throws, eg. due to main.js breaking, we fall back to 'error' }