Skip to content

Commit f60f763

Browse files
authored
fix(node): Remove ANR debug option and instead add logger.isEnabled() (#9230)
A separate `debug` option for `enableAnrDetection` is confusing and is not required if we add an `isEnabled()` function to the logger.
1 parent d1d7e79 commit f60f763

File tree

6 files changed

+32
-14
lines changed

6 files changed

+32
-14
lines changed

packages/node-integration-tests/suites/anr/basic.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,14 @@ setTimeout(() => {
1010
Sentry.init({
1111
dsn: 'https://[email protected]/1337',
1212
release: '1.0',
13+
debug: true,
1314
beforeSend: event => {
1415
// eslint-disable-next-line no-console
1516
console.log(JSON.stringify(event));
1617
},
1718
});
1819

19-
Sentry.enableAnrDetection({ captureStackTrace: true, anrThreshold: 200, debug: true }).then(() => {
20+
Sentry.enableAnrDetection({ captureStackTrace: true, anrThreshold: 200 }).then(() => {
2021
function longWork() {
2122
for (let i = 0; i < 100; i++) {
2223
const salt = crypto.randomBytes(128).toString('base64');

packages/node-integration-tests/suites/anr/basic.mjs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,14 @@ setTimeout(() => {
1010
Sentry.init({
1111
dsn: 'https://[email protected]/1337',
1212
release: '1.0',
13+
debug: true,
1314
beforeSend: event => {
1415
// eslint-disable-next-line no-console
1516
console.log(JSON.stringify(event));
1617
},
1718
});
1819

19-
await Sentry.enableAnrDetection({ captureStackTrace: true, anrThreshold: 200, debug: true });
20+
await Sentry.enableAnrDetection({ captureStackTrace: true, anrThreshold: 200 });
2021

2122
function longWork() {
2223
for (let i = 0; i < 100; i++) {

packages/node-integration-tests/suites/anr/forked.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,14 @@ setTimeout(() => {
1010
Sentry.init({
1111
dsn: 'https://[email protected]/1337',
1212
release: '1.0',
13+
debug: true,
1314
beforeSend: event => {
1415
// eslint-disable-next-line no-console
1516
console.log(JSON.stringify(event));
1617
},
1718
});
1819

19-
Sentry.enableAnrDetection({ captureStackTrace: true, anrThreshold: 200, debug: true }).then(() => {
20+
Sentry.enableAnrDetection({ captureStackTrace: true, anrThreshold: 200 }).then(() => {
2021
function longWork() {
2122
for (let i = 0; i < 100; i++) {
2223
const salt = crypto.randomBytes(128).toString('base64');

packages/node-integration-tests/suites/anr/test.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,22 @@ import * as path from 'path';
55

66
const NODE_VERSION = parseSemver(process.versions.node).major || 0;
77

8+
/** The output will contain logging so we need to find the line that parses as JSON */
9+
function parseJsonLine<T>(input: string): T {
10+
return (
11+
input
12+
.split('\n')
13+
.map(line => {
14+
try {
15+
return JSON.parse(line) as T;
16+
} catch {
17+
return undefined;
18+
}
19+
})
20+
.filter(a => a) as T[]
21+
)[0];
22+
}
23+
824
describe('should report ANR when event loop blocked', () => {
925
test('CJS', done => {
1026
// The stack trace is different when node < 12
@@ -15,7 +31,7 @@ describe('should report ANR when event loop blocked', () => {
1531
const testScriptPath = path.resolve(__dirname, 'basic.js');
1632

1733
childProcess.exec(`node ${testScriptPath}`, { encoding: 'utf8' }, (_, stdout) => {
18-
const event = JSON.parse(stdout) as Event;
34+
const event = parseJsonLine<Event>(stdout);
1935

2036
expect(event.exception?.values?.[0].mechanism).toEqual({ type: 'ANR' });
2137
expect(event.exception?.values?.[0].type).toEqual('ApplicationNotResponding');
@@ -42,7 +58,7 @@ describe('should report ANR when event loop blocked', () => {
4258
const testScriptPath = path.resolve(__dirname, 'basic.mjs');
4359

4460
childProcess.exec(`node ${testScriptPath}`, { encoding: 'utf8' }, (_, stdout) => {
45-
const event = JSON.parse(stdout) as Event;
61+
const event = parseJsonLine<Event>(stdout);
4662

4763
expect(event.exception?.values?.[0].mechanism).toEqual({ type: 'ANR' });
4864
expect(event.exception?.values?.[0].type).toEqual('ApplicationNotResponding');
@@ -64,7 +80,7 @@ describe('should report ANR when event loop blocked', () => {
6480
const testScriptPath = path.resolve(__dirname, 'forker.js');
6581

6682
childProcess.exec(`node ${testScriptPath}`, { encoding: 'utf8' }, (_, stdout) => {
67-
const event = JSON.parse(stdout) as Event;
83+
const event = parseJsonLine<Event>(stdout);
6884

6985
expect(event.exception?.values?.[0].mechanism).toEqual({ type: 'ANR' });
7086
expect(event.exception?.values?.[0].type).toEqual('ApplicationNotResponding');

packages/node/src/anr/index.ts

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ interface Options {
3636
*/
3737
captureStackTrace: boolean;
3838
/**
39-
* Log debug information.
39+
* @deprecated Use 'init' debug option instead
4040
*/
4141
debug: boolean;
4242
}
@@ -94,9 +94,7 @@ function startInspector(startPort: number = 9229): string | undefined {
9494

9595
function startChildProcess(options: Options): void {
9696
function log(message: string, ...args: unknown[]): void {
97-
if (options.debug) {
98-
logger.log(`[ANR] ${message}`, ...args);
99-
}
97+
logger.log(`[ANR] ${message}`, ...args);
10098
}
10199

102100
try {
@@ -111,7 +109,7 @@ function startChildProcess(options: Options): void {
111109

112110
const child = spawn(process.execPath, [options.entryScript], {
113111
env,
114-
stdio: options.debug ? ['inherit', 'inherit', 'inherit', 'ipc'] : ['ignore', 'ignore', 'ignore', 'ipc'],
112+
stdio: logger.isEnabled() ? ['inherit', 'inherit', 'inherit', 'ipc'] : ['ignore', 'ignore', 'ignore', 'ipc'],
115113
});
116114
// The child process should not keep the main process alive
117115
child.unref();
@@ -142,9 +140,7 @@ function startChildProcess(options: Options): void {
142140

143141
function handleChildProcess(options: Options): void {
144142
function log(message: string): void {
145-
if (options.debug) {
146-
logger.log(`[ANR child process] ${message}`);
147-
}
143+
logger.log(`[ANR child process] ${message}`);
148144
}
149145

150146
process.title = 'sentry-anr';
@@ -233,6 +229,7 @@ export function enableAnrDetection(options: Partial<Options>): Promise<void> {
233229
pollInterval: options.pollInterval || DEFAULT_INTERVAL,
234230
anrThreshold: options.anrThreshold || DEFAULT_HANG_THRESHOLD,
235231
captureStackTrace: !!options.captureStackTrace,
232+
// eslint-disable-next-line deprecation/deprecation
236233
debug: !!options.debug,
237234
};
238235

packages/utils/src/logger.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ export const originalConsoleMethods: {
1919
interface Logger extends LoggerConsoleMethods {
2020
disable(): void;
2121
enable(): void;
22+
isEnabled(): boolean;
2223
}
2324

2425
/**
@@ -63,6 +64,7 @@ function makeLogger(): Logger {
6364
disable: () => {
6465
enabled = false;
6566
},
67+
isEnabled: () => enabled,
6668
};
6769

6870
if (__DEBUG_BUILD__) {

0 commit comments

Comments
 (0)