Merge pull request #21416 from storybookjs/tom/21318-fix-unreported-e…

…rrors

Telemetry: Ensure we report errors even when unexpected things happen

code/lib/core-server/src/withTelemetry.test.ts

@@ -68,12 +68,12 @@ describe('when command fails', () => {
     expect(telemetry).toHaveBeenCalledTimes(0);
     expect(telemetry).not.toHaveBeenCalledWith(
       'error',
-      { eventType: 'dev', error },
+      expect.objectContaining({}),
       expect.objectContaining({})
     );
   });
 
-  it('does not send error message when crash reports are disabled', async () => {
+  it('does not send full error message when crash reports are disabled', async () => {
     jest.mocked(loadAllPresets).mockResolvedValueOnce({
       apply: async () => ({ enableCrashReports: false } as any),
     });
@@ -106,7 +106,7 @@ describe('when command fails', () => {
     );
   });
 
-  it('does not send full error message when telemetry is disabled', async () => {
+  it('does not send any error message when telemetry is disabled', async () => {
     jest.mocked(loadAllPresets).mockResolvedValueOnce({
       apply: async () => ({ disableTelemetry: true } as any),
     });
@@ -140,7 +140,7 @@ describe('when command fails', () => {
     );
   });
 
-  it('does not send error messages when disabled crash reports are cached', async () => {
+  it('does not send  full  error messages when disabled crash reports are cached', async () => {
     jest.mocked(loadAllPresets).mockResolvedValueOnce({
       apply: async () => ({} as any),
     });
@@ -176,7 +176,7 @@ describe('when command fails', () => {
     );
   });
 
-  it('does not send error messages when disabled crash reports are prompted', async () => {
+  it('does not send full error messages when disabled crash reports are prompted', async () => {
     jest.mocked(loadAllPresets).mockResolvedValueOnce({
       apply: async () => ({} as any),
     });
@@ -214,18 +214,19 @@ describe('when command fails', () => {
     );
   });
 
-  // 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 () => {
+  // if main.js has errors, we have no way to tell if they've disabled error reporting,
+  // so we assume they have.
+  it('does not send full error messages when presets fail to evaluate', async () => {
     jest.mocked(loadAllPresets).mockRejectedValueOnce(error);
 
     await expect(async () =>
       withTelemetry('dev', { cliOptions: {} as any, presetOptions: {} as any }, run)
     ).rejects.toThrow(error);
 
-    expect(telemetry).toHaveBeenCalledTimes(1);
-    expect(telemetry).not.toHaveBeenCalledWith(
+    expect(telemetry).toHaveBeenCalledTimes(2);
+    expect(telemetry).toHaveBeenCalledWith(
       'error',
-      { eventType: 'dev', error },
+      { eventType: 'dev' },
       expect.objectContaining({})
     );
   });

code/lib/core-server/src/withTelemetry.ts

@@ -64,7 +64,12 @@ export async function sendTelemetryError(
   options: TelemetryOptions
 ) {
   try {
-    const errorLevel = await getErrorLevel(options);
+    let errorLevel = 'error';
+    try {
+      errorLevel = await getErrorLevel(options);
+    } catch (err) {
+      // If this throws, eg. due to main.js breaking, we fall back to 'error'
+    }
     if (errorLevel !== 'none') {
       const precedingUpgrade = await getPrecedingUpgrade();
 
@@ -74,7 +79,7 @@ export async function sendTelemetryError(
           eventType,
           precedingUpgrade,
           error: errorLevel === 'full' ? error : undefined,
-          errorHash: oneWayHash(error.message),
+          errorHash: oneWayHash(error.message || ''),
         },
         {
           immediate: true,