Merge pull request #23919 from storybookjs/norbert/improve-node-logge…

…r-error Logger: Fix double error messages/stack
storybookjs · Aug 23, 2023 · 7830782 · 7830782
2 parents f0fe293 + 7e135d2
commit 7830782
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 6 deletions.
diff --git a/code/frameworks/angular/src/builders/utils/error-handler.ts b/code/frameworks/angular/src/builders/utils/error-handler.ts
@@ -12,7 +12,7 @@ export const printErrorDetails = (error: any): void => {
     } else if ((error as any).stats && (error as any).stats.compilation.errors) {
       (error as any).stats.compilation.errors.forEach((e: any) => logger.plain(e));
     } else {
-      logger.error(error as any);
+      logger.error(error);
     }
   } else if (error.compilation?.errors) {
     error.compilation.errors.forEach((e: any) => logger.plain(e));

diff --git a/code/lib/core-server/src/withTelemetry.ts b/code/lib/core-server/src/withTelemetry.ts
@@ -151,7 +151,7 @@ export async function withTelemetry<T>(
 
   try {
     return await run();
-  } catch (error) {
+  } catch (error: any) {
     if (canceled) {
       return undefined;
     }

diff --git a/code/lib/node-logger/src/index.test.ts b/code/lib/node-logger/src/index.test.ts
@@ -1,12 +1,28 @@
-import { info, warn, error } from 'npmlog';
+import { info, warn } from 'npmlog';
 import { logger } from '.';
 
+globalThis.console = { log: jest.fn() } as any;
+
 jest.mock('npmlog', () => ({
   info: jest.fn(),
   warn: jest.fn(),
   error: jest.fn(),
+  levels: {
+    silly: -Infinity,
+    verbose: 1000,
+    info: 2000,
+    timing: 2500,
+    http: 3000,
+    notice: 3500,
+    warn: 4000,
+    error: 5000,
+    silent: Infinity,
+  },
+  level: 'info',
 }));
 
+//
+
 describe('node-logger', () => {
   it('should have an info method', () => {
     const message = 'information';
@@ -21,6 +37,13 @@ describe('node-logger', () => {
   it('should have an error method', () => {
     const message = 'error message';
     logger.error(message);
-    expect(error).toHaveBeenCalledWith('', message);
+    expect(globalThis.console.log).toHaveBeenCalledWith(expect.stringMatching('message'));
+  });
+  it('should format errors', () => {
+    const message = new Error('A complete disaster');
+    logger.error(message);
+    expect(globalThis.console.log).toHaveBeenCalledWith(
+      expect.stringMatching('A complete disaster')
+    );
   });
 });
diff --git a/code/lib/node-logger/src/index.ts b/code/lib/node-logger/src/index.ts
@@ -25,13 +25,28 @@ export const logger = {
   plain: (message: string): void => console.log(message),
   line: (count = 1): void => console.log(`${Array(count - 1).fill('\n')}`),
   warn: (message: string): void => npmLog.warn('', message),
-  // npmLog supports anything we log, it will just stringify it
-  error: (message: unknown): void => npmLog.error('', message as string),
   trace: ({ message, time }: { message: string; time: [number, number] }): void =>
     npmLog.info('', `${message} (${colors.purple(prettyTime(time))})`),
   setLevel: (level = 'info'): void => {
     npmLog.level = level;
   },
+  error: (message: Error | string): void => {
+    if (npmLog.levels[npmLog.level] < npmLog.levels.error) {
+      let msg: string;
+
+      if (message instanceof Error && message.stack) {
+        msg = message.stack.toString();
+      } else {
+        msg = message.toString();
+      }
+
+      console.log(
+        msg
+          .replace(message.toString(), chalk.red(message.toString()))
+          .replaceAll(process.cwd(), '.')
+      );
+    }
+  },
 };
 
 export { npmLog as instance };