From de93a1cdab831042b981529d7a76974def92b8a2 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 12 Oct 2023 02:07:30 +0100 Subject: [PATCH] fix(node): Remove ANR debug option and use logger.isEnabled() --- .../suites/anr/basic.js | 3 ++- .../suites/anr/basic.mjs | 3 ++- .../suites/anr/forked.js | 3 ++- .../node-integration-tests/suites/anr/test.ts | 22 ++++++++++++++++--- packages/node/src/anr/index.ts | 13 +++++------ packages/utils/src/logger.ts | 2 ++ 6 files changed, 32 insertions(+), 14 deletions(-) diff --git a/packages/node-integration-tests/suites/anr/basic.js b/packages/node-integration-tests/suites/anr/basic.js index 3abadc09b9c3..45a324e507c5 100644 --- a/packages/node-integration-tests/suites/anr/basic.js +++ b/packages/node-integration-tests/suites/anr/basic.js @@ -10,13 +10,14 @@ setTimeout(() => { Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', + debug: true, beforeSend: event => { // eslint-disable-next-line no-console console.log(JSON.stringify(event)); }, }); -Sentry.enableAnrDetection({ captureStackTrace: true, anrThreshold: 200, debug: true }).then(() => { +Sentry.enableAnrDetection({ captureStackTrace: true, anrThreshold: 200 }).then(() => { function longWork() { for (let i = 0; i < 100; i++) { const salt = crypto.randomBytes(128).toString('base64'); diff --git a/packages/node-integration-tests/suites/anr/basic.mjs b/packages/node-integration-tests/suites/anr/basic.mjs index ba9c8623da7e..1d89ac1b3989 100644 --- a/packages/node-integration-tests/suites/anr/basic.mjs +++ b/packages/node-integration-tests/suites/anr/basic.mjs @@ -10,13 +10,14 @@ setTimeout(() => { Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', + debug: true, beforeSend: event => { // eslint-disable-next-line no-console console.log(JSON.stringify(event)); }, }); -await Sentry.enableAnrDetection({ captureStackTrace: true, anrThreshold: 200, debug: true }); +await Sentry.enableAnrDetection({ captureStackTrace: true, anrThreshold: 200 }); function longWork() { for (let i = 0; i < 100; i++) { diff --git a/packages/node-integration-tests/suites/anr/forked.js b/packages/node-integration-tests/suites/anr/forked.js index 3abadc09b9c3..45a324e507c5 100644 --- a/packages/node-integration-tests/suites/anr/forked.js +++ b/packages/node-integration-tests/suites/anr/forked.js @@ -10,13 +10,14 @@ setTimeout(() => { Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', + debug: true, beforeSend: event => { // eslint-disable-next-line no-console console.log(JSON.stringify(event)); }, }); -Sentry.enableAnrDetection({ captureStackTrace: true, anrThreshold: 200, debug: true }).then(() => { +Sentry.enableAnrDetection({ captureStackTrace: true, anrThreshold: 200 }).then(() => { function longWork() { for (let i = 0; i < 100; i++) { const salt = crypto.randomBytes(128).toString('base64'); diff --git a/packages/node-integration-tests/suites/anr/test.ts b/packages/node-integration-tests/suites/anr/test.ts index 4fd83c0b3205..96d83c64a6a7 100644 --- a/packages/node-integration-tests/suites/anr/test.ts +++ b/packages/node-integration-tests/suites/anr/test.ts @@ -5,6 +5,22 @@ import * as path from 'path'; const NODE_VERSION = parseSemver(process.versions.node).major || 0; +/** The output will contain logging so we need to find the line that parses as JSON */ +function parseJsonLine(input: string): T { + return ( + input + .split('\n') + .map(line => { + try { + return JSON.parse(line) as T; + } catch { + return undefined; + } + }) + .filter(a => a) as T[] + )[0]; +} + describe('should report ANR when event loop blocked', () => { test('CJS', done => { // The stack trace is different when node < 12 @@ -15,7 +31,7 @@ describe('should report ANR when event loop blocked', () => { const testScriptPath = path.resolve(__dirname, 'basic.js'); childProcess.exec(`node ${testScriptPath}`, { encoding: 'utf8' }, (_, stdout) => { - const event = JSON.parse(stdout) as Event; + const event = parseJsonLine(stdout); expect(event.exception?.values?.[0].mechanism).toEqual({ type: 'ANR' }); expect(event.exception?.values?.[0].type).toEqual('ApplicationNotResponding'); @@ -42,7 +58,7 @@ describe('should report ANR when event loop blocked', () => { const testScriptPath = path.resolve(__dirname, 'basic.mjs'); childProcess.exec(`node ${testScriptPath}`, { encoding: 'utf8' }, (_, stdout) => { - const event = JSON.parse(stdout) as Event; + const event = parseJsonLine(stdout); expect(event.exception?.values?.[0].mechanism).toEqual({ type: 'ANR' }); expect(event.exception?.values?.[0].type).toEqual('ApplicationNotResponding'); @@ -64,7 +80,7 @@ describe('should report ANR when event loop blocked', () => { const testScriptPath = path.resolve(__dirname, 'forker.js'); childProcess.exec(`node ${testScriptPath}`, { encoding: 'utf8' }, (_, stdout) => { - const event = JSON.parse(stdout) as Event; + const event = parseJsonLine(stdout); expect(event.exception?.values?.[0].mechanism).toEqual({ type: 'ANR' }); expect(event.exception?.values?.[0].type).toEqual('ApplicationNotResponding'); diff --git a/packages/node/src/anr/index.ts b/packages/node/src/anr/index.ts index 38aa697457dd..1d4d61b78b55 100644 --- a/packages/node/src/anr/index.ts +++ b/packages/node/src/anr/index.ts @@ -36,7 +36,7 @@ interface Options { */ captureStackTrace: boolean; /** - * Log debug information. + * @deprecated Use 'init' debug option instead */ debug: boolean; } @@ -94,9 +94,7 @@ function startInspector(startPort: number = 9229): string | undefined { function startChildProcess(options: Options): void { function log(message: string, ...args: unknown[]): void { - if (options.debug) { - logger.log(`[ANR] ${message}`, ...args); - } + logger.log(`[ANR] ${message}`, ...args); } try { @@ -111,7 +109,7 @@ function startChildProcess(options: Options): void { const child = spawn(process.execPath, [options.entryScript], { env, - stdio: options.debug ? ['inherit', 'inherit', 'inherit', 'ipc'] : ['ignore', 'ignore', 'ignore', 'ipc'], + stdio: logger.isEnabled() ? ['inherit', 'inherit', 'inherit', 'ipc'] : ['ignore', 'ignore', 'ignore', 'ipc'], }); // The child process should not keep the main process alive child.unref(); @@ -142,9 +140,7 @@ function startChildProcess(options: Options): void { function handleChildProcess(options: Options): void { function log(message: string): void { - if (options.debug) { - logger.log(`[ANR child process] ${message}`); - } + logger.log(`[ANR child process] ${message}`); } process.title = 'sentry-anr'; @@ -233,6 +229,7 @@ export function enableAnrDetection(options: Partial): Promise { pollInterval: options.pollInterval || DEFAULT_INTERVAL, anrThreshold: options.anrThreshold || DEFAULT_HANG_THRESHOLD, captureStackTrace: !!options.captureStackTrace, + // eslint-disable-next-line deprecation/deprecation debug: !!options.debug, }; diff --git a/packages/utils/src/logger.ts b/packages/utils/src/logger.ts index 07642b8c4f04..9d37855e9114 100644 --- a/packages/utils/src/logger.ts +++ b/packages/utils/src/logger.ts @@ -19,6 +19,7 @@ export const originalConsoleMethods: { interface Logger extends LoggerConsoleMethods { disable(): void; enable(): void; + isEnabled(): boolean; } /** @@ -63,6 +64,7 @@ function makeLogger(): Logger { disable: () => { enabled = false; }, + isEnabled: () => enabled, }; if (__DEBUG_BUILD__) {