From d93117339dc643ab19e5e8273a3b8704dfe36abe Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Tue, 26 Nov 2024 10:12:53 +0100 Subject: [PATCH 1/3] feat: Add support for OPEN_NEXT_ERROR_LOG_LEVEL OPEN_NEXT_ERROR_LOG_LEVEL is the minimal error level from which internal errors are logged. It can be set to - "0" / "debug" - "1" / "warn" (default) - "2" / "error" --- .changeset/chilly-cycles-own.md | 12 +++ packages/open-next/src/adapters/logger.ts | 57 ++++++++---- packages/open-next/src/utils/error.ts | 8 ++ .../tests-unit/tests/adapters/logger.test.ts | 92 +++++++++++++++++++ 4 files changed, 153 insertions(+), 16 deletions(-) create mode 100644 .changeset/chilly-cycles-own.md create mode 100644 packages/tests-unit/tests/adapters/logger.test.ts diff --git a/.changeset/chilly-cycles-own.md b/.changeset/chilly-cycles-own.md new file mode 100644 index 000000000..9b6b50052 --- /dev/null +++ b/.changeset/chilly-cycles-own.md @@ -0,0 +1,12 @@ +--- +"@opennextjs/aws": minor +--- + +feat: Add support for OPEN_NEXT_ERROR_LOG_LEVEL + +OPEN_NEXT_ERROR_LOG_LEVEL is the minimal error level from which internal errors are logged. +It can be set to: + +- "0" / "debug" +- "1" / "warn" (default) +- "2" / "error" diff --git a/packages/open-next/src/adapters/logger.ts b/packages/open-next/src/adapters/logger.ts index 7571a4f9e..a25b893a4 100644 --- a/packages/open-next/src/adapters/logger.ts +++ b/packages/open-next/src/adapters/logger.ts @@ -1,4 +1,4 @@ -import type { BaseOpenNextError } from "utils/error"; +import { type BaseOpenNextError, isOpenNextError } from "utils/error"; export function debug(...args: any[]) { if (globalThis.openNextDebug) { @@ -42,28 +42,33 @@ const isDownplayedErrorLog = (errorLog: AwsSdkClientCommandErrorLog) => export function error(...args: any[]) { // we try to catch errors from the aws-sdk client and downplay some of them - if ( - args.some((arg: AwsSdkClientCommandErrorLog) => isDownplayedErrorLog(arg)) - ) { + if (args.some((arg) => isDownplayedErrorLog(arg))) { debug(...args); - } else if (args.some((arg) => arg.__openNextInternal)) { + } else if (args.some((arg) => isOpenNextError(arg))) { // In case of an internal error, we log it with the appropriate log level - const error = args.find( - (arg) => arg.__openNextInternal, - ) as BaseOpenNextError; - if (error.logLevel === 0) { - debug(...args); + const error = args.find((arg) => isOpenNextError(arg))!; + if (error.logLevel < getOpenNextErrorLogLevel()) { return; } + if (error.logLevel === 0) { + // Display the name and the message instead of full Open Next errors. + return debug( + ...args.map((arg) => + isOpenNextError(arg) ? `${arg.name}: ${arg.message}` : arg, + ), + ); + } if (error.logLevel === 1) { - warn(...args); - return; + // Display the name and the message instead of full Open Next errors. + return warn( + ...args.map((arg) => + isOpenNextError(arg) ? `${arg.name}: ${arg.message}` : arg, + ), + ); } - console.error(...args); - return; - } else { - console.error(...args); + return console.error(...args); } + console.error(...args); } export const awsLogger = { @@ -73,3 +78,23 @@ export const awsLogger = { warn, error, }; + +/** + * Retrieves the log level for internal errors from the + * OPEN_NEXT_ERROR_LOG_LEVEL environment variable. + * + * @returns The numerical log level 0 (debug), 1 (warn), or 2 (error) + */ +function getOpenNextErrorLogLevel(): number { + const strLevel = process.env.OPEN_NEXT_ERROR_LOG_LEVEL ?? "1"; + switch (strLevel.toLowerCase()) { + case "debug": + case "0": + return 0; + case "error": + case "2": + return 2; + default: + return 1; + } +} diff --git a/packages/open-next/src/utils/error.ts b/packages/open-next/src/utils/error.ts index 2aae61c7b..c1e8dc9c0 100644 --- a/packages/open-next/src/utils/error.ts +++ b/packages/open-next/src/utils/error.ts @@ -39,3 +39,11 @@ export class FatalError extends Error implements BaseOpenNextError { this.name = "FatalError"; } } + +export function isOpenNextError(e: any): e is BaseOpenNextError & Error { + try { + return "__openNextInternal" in e; + } catch { + return false; + } +} diff --git a/packages/tests-unit/tests/adapters/logger.test.ts b/packages/tests-unit/tests/adapters/logger.test.ts new file mode 100644 index 000000000..6ed11d009 --- /dev/null +++ b/packages/tests-unit/tests/adapters/logger.test.ts @@ -0,0 +1,92 @@ +import * as logger from "@opennextjs/aws/adapters/logger.js"; +import { + FatalError, + IgnorableError, + RecoverableError, +} from "@opennextjs/aws/utils/error.js"; +import { vi } from "vitest"; + +describe("logger adapter", () => { + describe("Open Next errors", () => { + const debug = vi.spyOn(console, "log").mockImplementation(() => null); + const warn = vi.spyOn(console, "warn").mockImplementation(() => null); + const error = vi.spyOn(console, "error").mockImplementation(() => null); + + beforeEach(() => { + // Direct debug to console.log + globalThis.openNextDebug = true; + debug.mockClear(); + warn.mockClear(); + error.mockClear(); + }); + + afterAll(() => { + globalThis.openNextDebug = false; + }); + + const ignorableError = new IgnorableError("ignorable"); + const recoverableError = new RecoverableError("recoverable"); + const fatalError = new FatalError("fatal"); + + it("default to warn when OPEN_NEXT_ERROR_LOG_LEVEL is undefined", () => { + delete process.env.OPEN_NEXT_ERROR_LOG_LEVEL; + logger.error(ignorableError); + logger.error(recoverableError); + logger.error(fatalError); + expect(debug).not.toHaveBeenCalled(); + expect(warn).toHaveBeenCalledWith("RecoverableError: recoverable"); + expect(error).toHaveBeenCalledWith(fatalError); + }); + + it("OPEN_NEXT_ERROR_LOG_LEVEL is 'debug'/'0'", () => { + process.env.OPEN_NEXT_ERROR_LOG_LEVEL = "0"; + logger.error(ignorableError); + logger.error(recoverableError); + logger.error(fatalError); + expect(debug).toHaveBeenCalledWith("IgnorableError: ignorable"); + expect(warn).toHaveBeenCalledWith("RecoverableError: recoverable"); + expect(error).toHaveBeenCalledWith(fatalError); + process.env.OPEN_NEXT_ERROR_LOG_LEVEL = "debug"; + logger.error(ignorableError); + logger.error(recoverableError); + logger.error(fatalError); + expect(debug).toHaveBeenCalledWith("IgnorableError: ignorable"); + expect(warn).toHaveBeenCalledWith("RecoverableError: recoverable"); + expect(error).toHaveBeenCalledWith(fatalError); + }); + + it("OPEN_NEXT_ERROR_LOG_LEVEL is 'warn'/'1'", () => { + process.env.OPEN_NEXT_ERROR_LOG_LEVEL = "1"; + logger.error(ignorableError); + logger.error(recoverableError); + logger.error(fatalError); + expect(debug).not.toHaveBeenCalled(); + expect(warn).toHaveBeenCalledWith("RecoverableError: recoverable"); + expect(error).toHaveBeenCalledWith(fatalError); + process.env.OPEN_NEXT_ERROR_LOG_LEVEL = "warn"; + logger.error(ignorableError); + logger.error(recoverableError); + logger.error(fatalError); + expect(debug).not.toHaveBeenCalled(); + expect(warn).toHaveBeenCalledWith("RecoverableError: recoverable"); + expect(error).toHaveBeenCalledWith(fatalError); + }); + + it("OPEN_NEXT_ERROR_LOG_LEVEL is 'error'/'2'", () => { + process.env.OPEN_NEXT_ERROR_LOG_LEVEL = "2"; + logger.error(ignorableError); + logger.error(recoverableError); + logger.error(fatalError); + expect(debug).not.toHaveBeenCalled(); + expect(warn).not.toHaveBeenCalled(); + expect(error).toHaveBeenCalledWith(fatalError); + process.env.OPEN_NEXT_ERROR_LOG_LEVEL = "error"; + logger.error(ignorableError); + logger.error(recoverableError); + logger.error(fatalError); + expect(debug).not.toHaveBeenCalled(); + expect(warn).not.toHaveBeenCalled(); + expect(error).toHaveBeenCalledWith(fatalError); + }); + }); +}); From 9bc7b1b841fa777ac2cea99af22ac87d70c58f69 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Tue, 26 Nov 2024 10:13:05 +0100 Subject: [PATCH 2/3] refactor: code cleanup --- packages/tests-unit/tests/adapters/cache.test.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/tests-unit/tests/adapters/cache.test.ts b/packages/tests-unit/tests/adapters/cache.test.ts index 242c4cb53..f5624b8ca 100644 --- a/packages/tests-unit/tests/adapters/cache.test.ts +++ b/packages/tests-unit/tests/adapters/cache.test.ts @@ -70,10 +70,7 @@ describe("S3Cache", () => { beforeEach(() => { vi.clearAllMocks(); - cache = new S3Cache({ - _appDir: false, - _requestHeaders: undefined as never, - }); + cache = new S3Cache(); globalThis.disableIncrementalCache = false; globalThis.isNextAfter15 = false; From 1f9cbe83a5e61f939afb845d286b90dd61f6ba36 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Tue, 26 Nov 2024 10:56:14 +0100 Subject: [PATCH 3/3] fixup! do not depend on openNextDebug --- packages/open-next/src/adapters/logger.ts | 3 ++- packages/tests-unit/tests/adapters/logger.test.ts | 6 ------ 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/packages/open-next/src/adapters/logger.ts b/packages/open-next/src/adapters/logger.ts index a25b893a4..b037dbd38 100644 --- a/packages/open-next/src/adapters/logger.ts +++ b/packages/open-next/src/adapters/logger.ts @@ -52,7 +52,8 @@ export function error(...args: any[]) { } if (error.logLevel === 0) { // Display the name and the message instead of full Open Next errors. - return debug( + // console.log is used so that logging does not depend on openNextDebug. + return console.log( ...args.map((arg) => isOpenNextError(arg) ? `${arg.name}: ${arg.message}` : arg, ), diff --git a/packages/tests-unit/tests/adapters/logger.test.ts b/packages/tests-unit/tests/adapters/logger.test.ts index 6ed11d009..8491d1699 100644 --- a/packages/tests-unit/tests/adapters/logger.test.ts +++ b/packages/tests-unit/tests/adapters/logger.test.ts @@ -13,17 +13,11 @@ describe("logger adapter", () => { const error = vi.spyOn(console, "error").mockImplementation(() => null); beforeEach(() => { - // Direct debug to console.log - globalThis.openNextDebug = true; debug.mockClear(); warn.mockClear(); error.mockClear(); }); - afterAll(() => { - globalThis.openNextDebug = false; - }); - const ignorableError = new IgnorableError("ignorable"); const recoverableError = new RecoverableError("recoverable"); const fatalError = new FatalError("fatal");