From c3f582375b4f55c6474176b197230e6b44a4ad79 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Sat, 30 Sep 2023 14:27:08 +0200 Subject: [PATCH] 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 --- packages/logger/src/utils/format.ts | 2 +- .../logger/test/fixtures/loggerFormats.ts | 22 ++++++++++++++++--- packages/logger/test/unit/browser.test.ts | 9 ++++++-- packages/logger/test/unit/env.node.test.ts | 4 ++-- packages/logger/test/unit/node.node.test.ts | 3 ++- 5 files changed, 31 insertions(+), 9 deletions(-) diff --git a/packages/logger/src/utils/format.ts b/packages/logger/src/utils/format.ts index 18575b16e246..96e5893b71a0 100644 --- 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 index b9e0caf7bfa8..1138cd7cb3de 100644 --- 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 index c6924404bd97..c1dd70b6bebd 100644 --- 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 index f03567d04bf3..547f891b7ea1 100644 --- a/packages/logger/test/unit/env.node.test.ts +++ b/packages/logger/test/unit/env.node.test.ts @@ -8,7 +8,7 @@ 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 @@ -16,7 +16,7 @@ describe("env logger", () => { 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 index 41c6f8f8f620..6342ae9e4ccb 100644 --- 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}, }) );