Skip to content

Commit

Permalink
Merge pull request #20168 from storybookjs/revert-20136-shilman/prece…
Browse files Browse the repository at this point in the history
…ding-upgrade-event

Revert "Telemetry: Add precedingUpgrade data to dev/build/error events"
  • Loading branch information
shilman authored Dec 8, 2022
2 parents f5aa7bd + 55010a5 commit dddb25b
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 290 deletions.
15 changes: 6 additions & 9 deletions code/lib/core-server/src/build-static.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { dedent } from 'ts-dedent';
import global from 'global';

import { logger } from '@storybook/node-logger';
import { telemetry, getPrecedingUpgrade } from '@storybook/telemetry';
import { telemetry } from '@storybook/telemetry';
import type {
BuilderOptions,
CLIOptions,
Expand Down Expand Up @@ -173,14 +173,11 @@ export async function buildStaticStandalone(
effects.push(
initializedStoryIndexGenerator.then(async (generator) => {
const storyIndex = await generator?.getIndex();
const payload = {
precedingUpgrade: await getPrecedingUpgrade(),
};
if (storyIndex) {
Object.assign(payload, {
storyIndex: summarizeIndex(storyIndex),
});
}
const payload = storyIndex
? {
storyIndex: summarizeIndex(storyIndex),
}
: undefined;
await telemetry('build', payload, { configDir: options.configDir });
})
);
Expand Down
17 changes: 7 additions & 10 deletions code/lib/core-server/src/utils/doTelemetry.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { CoreConfig, Options } from '@storybook/types';
import { telemetry, getPrecedingUpgrade } from '@storybook/telemetry';
import { telemetry } from '@storybook/telemetry';
import { useStorybookMetadata } from './metadata';
import type { StoryIndexGenerator } from './StoryIndexGenerator';
import { summarizeIndex } from './summarizeIndex';
Expand All @@ -15,15 +15,12 @@ export async function doTelemetry(
initializedStoryIndexGenerator.then(async (generator) => {
const storyIndex = await generator?.getIndex();
const { versionCheck, versionUpdates } = options;
const payload = {
precedingUpgrade: await getPrecedingUpgrade(),
};
if (storyIndex) {
Object.assign(payload, {
versionStatus: versionUpdates ? versionStatus(versionCheck) : 'disabled',
storyIndex: summarizeIndex(storyIndex),
});
}
const payload = storyIndex
? {
versionStatus: versionUpdates ? versionStatus(versionCheck) : 'disabled',
storyIndex: summarizeIndex(storyIndex),
}
: undefined;
telemetry('dev', payload, { configDir: options.configDir });
});
}
Expand Down
20 changes: 10 additions & 10 deletions code/lib/core-server/src/withTelemetry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,10 @@ describe('when command fails', () => {
withTelemetry('dev', { presetOptions: {} as any }, run)
).rejects.toThrow(error);

expect(telemetry).toHaveBeenCalledTimes(2);
expect(telemetry).toHaveBeenCalledWith(
expect(telemetry).toHaveBeenCalledTimes(1);
expect(telemetry).not.toHaveBeenCalledWith(
'error',
{ eventType: 'dev' },
{ eventType: 'dev', error },
expect.objectContaining({})
);
});
Expand All @@ -88,7 +88,7 @@ describe('when command fails', () => {
);
});

it('does not send full error message when telemetry is disabled', async () => {
it('does not send error message when telemetry is disabled', async () => {
jest.mocked(loadAllPresets).mockResolvedValueOnce({
apply: async () => ({ disableTelemetry: true } as any),
});
Expand Down Expand Up @@ -132,10 +132,10 @@ describe('when command fails', () => {
withTelemetry('dev', { presetOptions: {} as any }, run)
).rejects.toThrow(error);

expect(telemetry).toHaveBeenCalledTimes(2);
expect(telemetry).toHaveBeenCalledWith(
expect(telemetry).toHaveBeenCalledTimes(1);
expect(telemetry).not.toHaveBeenCalledWith(
'error',
{ eventType: 'dev' },
{ eventType: 'dev', error },
expect.objectContaining({})
);
});
Expand Down Expand Up @@ -169,10 +169,10 @@ describe('when command fails', () => {
withTelemetry('dev', { presetOptions: {} as any }, run)
).rejects.toThrow(error);

expect(telemetry).toHaveBeenCalledTimes(2);
expect(telemetry).toHaveBeenCalledWith(
expect(telemetry).toHaveBeenCalledTimes(1);
expect(telemetry).not.toHaveBeenCalledWith(
'error',
{ eventType: 'dev' },
{ eventType: 'dev', error },
expect.objectContaining({})
);
});
Expand Down
29 changes: 12 additions & 17 deletions code/lib/core-server/src/withTelemetry.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import prompts from 'prompts';
import type { CLIOptions, CoreConfig } from '@storybook/types';
import { loadAllPresets, cache } from '@storybook/core-common';
import { telemetry, getPrecedingUpgrade } from '@storybook/telemetry';
import { telemetry } from '@storybook/telemetry';
import type { EventType } from '@storybook/telemetry';

type TelemetryOptions = {
Expand All @@ -26,13 +26,11 @@ const promptCrashReports = async () => {
return enableCrashReports;
};

type ErrorLevel = 'none' | 'error' | 'full';

async function getErrorLevel({ cliOptions, presetOptions }: TelemetryOptions): Promise<ErrorLevel> {
if (cliOptions?.disableTelemetry) return 'none';
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 'full';
if (!presetOptions) return true;

// should we load the preset?
const presets = await loadAllPresets({
Expand All @@ -44,18 +42,18 @@ async function getErrorLevel({ cliOptions, presetOptions }: TelemetryOptions): P
// 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<CoreConfig>('core');
if (core?.enableCrashReports !== undefined) return core.enableCrashReports ? 'full' : 'error';
if (core?.disableTelemetry) return 'none';
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 ? 'full' : 'error';
if (valueFromCache !== undefined) return valueFromCache;

const valueFromPrompt = await promptCrashReports();
if (valueFromPrompt !== undefined) return valueFromPrompt ? 'full' : 'error';
if (valueFromPrompt !== undefined) return valueFromPrompt;

return 'full';
return true;
}

export async function withTelemetry(
Expand All @@ -69,17 +67,14 @@ export async function withTelemetry(
await run();
} catch (error) {
try {
const errorLevel = await getErrorLevel(options);
if (errorLevel !== 'none') {
const precedingUpgrade = await getPrecedingUpgrade();

if (await shouldSendError(options)) {
await telemetry(
'error',
{ eventType, precedingUpgrade, error: errorLevel === 'full' ? error : undefined },
{ eventType, error },
{
immediate: true,
configDir: options.cliOptions?.configDir || options.presetOptions?.configDir,
enableCrashReports: errorLevel === 'full',
enableCrashReports: true,
}
);
}
Expand Down
175 changes: 0 additions & 175 deletions code/lib/telemetry/src/event-cache.test.ts

This file was deleted.

Loading

0 comments on commit dddb25b

Please sign in to comment.