fix: more clearly separate log and error message (#5992)

* fix: more clearly separate log and error message * Use " - " separator in all cases and update test case * Re-add extra space before return * Remove unused import
ChainSafe · Sep 30, 2023 · c3f5823 · c3f5823
1 parent 3cfa9cd
commit c3f5823
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 9 deletions.
diff --git a/packages/logger/src/utils/format.ts b/packages/logger/src/utils/format.ts
@@ -87,7 +87,7 @@ function humanReadableTemplateFn(_info: {[key: string]: any; level: string; mess
   str += `[${infoString}] ${info.level.padStart(infoPad)}: ${info.message}`;
 
   if (info.context !== undefined) str += " " + logCtxToString(info.context);
-  if (info.error !== undefined) str += " " + logCtxToString(info.error);
+  if (info.error !== undefined) str += " - " + logCtxToString(info.error);
 
   return str;
 }
diff --git a/packages/logger/test/fixtures/loggerFormats.ts b/packages/logger/test/fixtures/loggerFormats.ts
@@ -3,6 +3,7 @@ import {LogData, LogFormat} from "../../src/index.js";
 
 type TestCase = {
   id: string;
+  opts?: {module?: string};
   message: string;
   context?: LogData;
   error?: Error;
@@ -31,17 +32,32 @@ export const formatsTestCases: (TestCase | (() => TestCase))[] = [
     },
   },
 
+  () => {
+    const error = new Error("err message");
+    error.stack = "$STACK";
+    return {
+      id: "regular log with error",
+      opts: {module: "test"},
+      message: "foo bar",
+      error: error,
+      output: {
+        human: `[test]             \u001b[33mwarn\u001b[39m: foo bar - err message\n${error.stack}`,
+        json: '{"error":{"message":"err message","stack":"$STACK"},"level":"warn","message":"foo bar","module":"test"}',
+      },
+    };
+  },
+
   () => {
     const error = new LodestarError({code: "SAMPLE_ERROR", data: {foo: "bar"}});
     error.stack = "$STACK";
     return {
       id: "error with metadata",
-      opts: {format: "human", module: "SAMPLE"},
+      opts: {module: "test"},
       message: "foo bar",
       error: error,
       output: {
-        human: `[]                 \u001b[33mwarn\u001b[39m: foo bar code=SAMPLE_ERROR, data=foo=bar\n${error.stack}`,
-        json: '{"error":{"code":"SAMPLE_ERROR","data":{"foo":"bar"},"stack":"$STACK"},"level":"warn","message":"foo bar","module":""}',
+        human: `[test]             \u001b[33mwarn\u001b[39m: foo bar - code=SAMPLE_ERROR, data=foo=bar\n${error.stack}`,
+        json: '{"error":{"code":"SAMPLE_ERROR","data":{"foo":"bar"},"stack":"$STACK"},"level":"warn","message":"foo bar","module":"test"}',
       },
     };
   },

diff --git a/packages/logger/test/unit/browser.test.ts b/packages/logger/test/unit/browser.test.ts
@@ -8,11 +8,16 @@ import {getBrowserLogger} from "../../src/browser.js";
 describe("browser logger", () => {
   describe("format and options", () => {
     for (const testCase of formatsTestCases) {
-      const {id, message, context, error, output} = typeof testCase === "function" ? testCase() : testCase;
+      const {id, opts, message, context, error, output} = typeof testCase === "function" ? testCase() : testCase;
       for (const format of logFormats) {
         it(`${id} ${format} output`, async () => {
           const logger = stubLoggerForConsole(
-            getBrowserLogger({level: LogLevel.info, format, timestampFormat: {format: TimestampFormatCode.Hidden}})
+            getBrowserLogger({
+              level: LogLevel.info,
+              format,
+              module: opts?.module,
+              timestampFormat: {format: TimestampFormatCode.Hidden},
+            })
           );
 
           logger.warn(message, context, error);

diff --git a/packages/logger/test/unit/env.node.test.ts b/packages/logger/test/unit/env.node.test.ts
@@ -8,15 +8,15 @@ import {getEnvLogger} from "../../src/env.js";
 describe("env logger", () => {
   describe("format and options", () => {
     for (const testCase of formatsTestCases) {
-      const {id, message, context, error, output} = typeof testCase === "function" ? testCase() : testCase;
+      const {id, opts, message, context, error, output} = typeof testCase === "function" ? testCase() : testCase;
       for (const format of logFormats) {
         it(`${id} ${format} output`, async () => {
           // Set env variables
           process.env.LOG_LEVEL = LogLevel.info;
           process.env.LOG_FORMAT = format;
           process.env.LOG_TIMESTAMP_FORMAT = TimestampFormatCode.Hidden;
 
-          const logger = stubLoggerForConsole(getEnvLogger());
+          const logger = stubLoggerForConsole(getEnvLogger({module: opts?.module}));
 
           logger.warn(message, context, error);
           logger.restoreStubs();

diff --git a/packages/logger/test/unit/node.node.test.ts b/packages/logger/test/unit/node.node.test.ts
@@ -8,13 +8,14 @@ import {formatsTestCases} from "../fixtures/loggerFormats.js";
 describe("node logger", () => {
   describe("format and options", () => {
     for (const testCase of formatsTestCases) {
-      const {id, message, context, error, output} = typeof testCase === "function" ? testCase() : testCase;
+      const {id, opts, message, context, error, output} = typeof testCase === "function" ? testCase() : testCase;
       for (const format of logFormats) {
         it(`${id} ${format} output`, async () => {
           const logger = stubLoggerForProcessStd(
             getNodeLogger({
               level: LogLevel.info,
               format,
+              module: opts?.module,
               timestampFormat: {format: TimestampFormatCode.Hidden},
             })
           );