Skip to content

Commit f94109c

Browse files
authored
Merge branch 'develop' into sig/inline-nitro-utils
2 parents c7095a4 + c49b247 commit f94109c

File tree

8 files changed

+162
-26
lines changed

8 files changed

+162
-26
lines changed

dev-packages/browser-integration-tests/suites/integrations/captureConsole/test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ sentryTest('it captures console messages correctly', async ({ getLocalTestUrl, p
3030
extra: {
3131
arguments: ['console log'],
3232
},
33+
message: 'console log',
3334
}),
3435
);
3536
expect(logEvent?.exception).toBeUndefined();
@@ -40,6 +41,7 @@ sentryTest('it captures console messages correctly', async ({ getLocalTestUrl, p
4041
extra: {
4142
arguments: ['console warn'],
4243
},
44+
message: 'console warn',
4345
}),
4446
);
4547
expect(warnEvent?.exception).toBeUndefined();
@@ -50,6 +52,7 @@ sentryTest('it captures console messages correctly', async ({ getLocalTestUrl, p
5052
extra: {
5153
arguments: ['console info'],
5254
},
55+
message: 'console info',
5356
}),
5457
);
5558
expect(infoEvent?.exception).toBeUndefined();
@@ -60,6 +63,7 @@ sentryTest('it captures console messages correctly', async ({ getLocalTestUrl, p
6063
extra: {
6164
arguments: ['console error'],
6265
},
66+
message: 'console error',
6367
}),
6468
);
6569
expect(errorEvent?.exception).toBeUndefined();
@@ -70,6 +74,7 @@ sentryTest('it captures console messages correctly', async ({ getLocalTestUrl, p
7074
extra: {
7175
arguments: ['console trace'],
7276
},
77+
message: 'console trace',
7378
}),
7479
);
7580
expect(traceEvent?.exception).toBeUndefined();
@@ -90,6 +95,11 @@ sentryTest('it captures console messages correctly', async ({ getLocalTestUrl, p
9095
}),
9196
);
9297
expect(errorWithErrorEvent?.exception?.values?.[0].value).toBe('console error with error object');
98+
expect(errorWithErrorEvent?.exception?.values?.[0].mechanism).toEqual({
99+
// TODO (v9): Adjust to true after changing the integration's default value
100+
handled: false,
101+
type: 'console',
102+
});
93103
expect(traceWithErrorEvent).toEqual(
94104
expect.objectContaining({
95105
level: 'log',

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

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,35 @@ const ANR_EVENT = {
5252
},
5353
};
5454

55+
const ANR_EVENT_WITHOUT_STACKTRACE = {
56+
// Ensure we have context
57+
contexts: {
58+
device: {
59+
arch: expect.any(String),
60+
},
61+
app: {
62+
app_start_time: expect.any(String),
63+
},
64+
os: {
65+
name: expect.any(String),
66+
},
67+
culture: {
68+
timezone: expect.any(String),
69+
},
70+
},
71+
// and an exception that is our ANR
72+
exception: {
73+
values: [
74+
{
75+
type: 'ApplicationNotResponding',
76+
value: 'Application Not Responding for at least 100 ms',
77+
mechanism: { type: 'ANR' },
78+
stacktrace: {},
79+
},
80+
],
81+
},
82+
};
83+
5584
const ANR_EVENT_WITH_SCOPE = {
5685
...ANR_EVENT,
5786
user: {
@@ -98,11 +127,11 @@ conditionalTest({ min: 16 })('should report ANR when event loop blocked', () =>
98127
createRunner(__dirname, 'indefinite.mjs').withMockSentryServer().expect({ event: ANR_EVENT }).start(done);
99128
});
100129

101-
test('With --inspect', done => {
130+
test("With --inspect the debugger isn't used", done => {
102131
createRunner(__dirname, 'basic.mjs')
103132
.withMockSentryServer()
104133
.withFlags('--inspect')
105-
.expect({ event: ANR_EVENT_WITH_SCOPE })
134+
.expect({ event: ANR_EVENT_WITHOUT_STACKTRACE })
106135
.start(done);
107136
});
108137

packages/core/src/integrations/captureconsole.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,24 @@ import { GLOBAL_OBJ } from '../utils-hoist/worldwide';
1111

1212
interface CaptureConsoleOptions {
1313
levels?: string[];
14+
15+
// TODO(v9): Flip default value to `true` and adjust JSDoc!
16+
/**
17+
* By default, Sentry will mark captured console messages as unhandled.
18+
* Set this to `true` if you want to mark them as handled instead.
19+
*
20+
* Note: in v9 of the SDK, this option will default to `true`, meaning the default behavior will change to mark console messages as handled.
21+
* @default false
22+
*/
23+
handled?: boolean;
1424
}
1525

1626
const INTEGRATION_NAME = 'CaptureConsole';
1727

1828
const _captureConsoleIntegration = ((options: CaptureConsoleOptions = {}) => {
1929
const levels = options.levels || CONSOLE_LEVELS;
30+
// TODO(v9): Flip default value to `true`
31+
const handled = !!options.handled;
2032

2133
return {
2234
name: INTEGRATION_NAME,
@@ -30,7 +42,7 @@ const _captureConsoleIntegration = ((options: CaptureConsoleOptions = {}) => {
3042
return;
3143
}
3244

33-
consoleHandler(args, level);
45+
consoleHandler(args, level, handled);
3446
});
3547
},
3648
};
@@ -41,7 +53,7 @@ const _captureConsoleIntegration = ((options: CaptureConsoleOptions = {}) => {
4153
*/
4254
export const captureConsoleIntegration = defineIntegration(_captureConsoleIntegration);
4355

44-
function consoleHandler(args: unknown[], level: string): void {
56+
function consoleHandler(args: unknown[], level: string, handled: boolean): void {
4557
const captureContext: CaptureContext = {
4658
level: severityLevelFromString(level),
4759
extra: {
@@ -54,7 +66,7 @@ function consoleHandler(args: unknown[], level: string): void {
5466
event.logger = 'console';
5567

5668
addExceptionMechanism(event, {
57-
handled: false,
69+
handled,
5870
type: 'console',
5971
});
6072

packages/core/test/lib/integrations/captureconsole.test.ts

Lines changed: 67 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -305,29 +305,78 @@ describe('CaptureConsole setup', () => {
305305
}).not.toThrow();
306306
});
307307

308-
it("marks captured exception's mechanism as unhandled", () => {
309-
// const addExceptionMechanismSpy = jest.spyOn(utils, 'addExceptionMechanism');
308+
describe('exception mechanism', () => {
309+
// TODO (v9): Flip this below after adjusting the default value for `handled` in the integration
310+
it("marks captured exception's mechanism as unhandled by default", () => {
311+
const captureConsole = captureConsoleIntegration({ levels: ['error'] });
312+
captureConsole.setup?.(mockClient);
310313

311-
const captureConsole = captureConsoleIntegration({ levels: ['error'] });
312-
captureConsole.setup?.(mockClient);
314+
const someError = new Error('some error');
315+
GLOBAL_OBJ.console.error(someError);
313316

314-
const someError = new Error('some error');
315-
GLOBAL_OBJ.console.error(someError);
317+
const addedEventProcessor = (mockScope.addEventProcessor as jest.Mock).mock.calls[0][0];
318+
const someEvent: Event = {
319+
exception: {
320+
values: [{}],
321+
},
322+
};
323+
addedEventProcessor(someEvent);
316324

317-
const addedEventProcessor = (mockScope.addEventProcessor as jest.Mock).mock.calls[0][0];
318-
const someEvent: Event = {
319-
exception: {
320-
values: [{}],
321-
},
322-
};
323-
addedEventProcessor(someEvent);
325+
expect(captureException).toHaveBeenCalledTimes(1);
326+
expect(mockScope.addEventProcessor).toHaveBeenCalledTimes(1);
324327

325-
expect(captureException).toHaveBeenCalledTimes(1);
326-
expect(mockScope.addEventProcessor).toHaveBeenCalledTimes(1);
328+
expect(someEvent.exception?.values?.[0]?.mechanism).toEqual({
329+
handled: false,
330+
type: 'console',
331+
});
332+
});
333+
334+
it("marks captured exception's mechanism as handled if set in the options", () => {
335+
const captureConsole = captureConsoleIntegration({ levels: ['error'], handled: true });
336+
captureConsole.setup?.(mockClient);
327337

328-
expect(someEvent.exception?.values?.[0]?.mechanism).toEqual({
329-
handled: false,
330-
type: 'console',
338+
const someError = new Error('some error');
339+
GLOBAL_OBJ.console.error(someError);
340+
341+
const addedEventProcessor = (mockScope.addEventProcessor as jest.Mock).mock.calls[0][0];
342+
const someEvent: Event = {
343+
exception: {
344+
values: [{}],
345+
},
346+
};
347+
addedEventProcessor(someEvent);
348+
349+
expect(captureException).toHaveBeenCalledTimes(1);
350+
expect(mockScope.addEventProcessor).toHaveBeenCalledTimes(1);
351+
352+
expect(someEvent.exception?.values?.[0]?.mechanism).toEqual({
353+
handled: true,
354+
type: 'console',
355+
});
356+
});
357+
358+
it("marks captured exception's mechanism as unhandled if set in the options", () => {
359+
const captureConsole = captureConsoleIntegration({ levels: ['error'], handled: false });
360+
captureConsole.setup?.(mockClient);
361+
362+
const someError = new Error('some error');
363+
GLOBAL_OBJ.console.error(someError);
364+
365+
const addedEventProcessor = (mockScope.addEventProcessor as jest.Mock).mock.calls[0][0];
366+
const someEvent: Event = {
367+
exception: {
368+
values: [{}],
369+
},
370+
};
371+
addedEventProcessor(someEvent);
372+
373+
expect(captureException).toHaveBeenCalledTimes(1);
374+
expect(mockScope.addEventProcessor).toHaveBeenCalledTimes(1);
375+
376+
expect(someEvent.exception?.values?.[0]?.mechanism).toEqual({
377+
handled: false,
378+
type: 'console',
379+
});
331380
});
332381
});
333382
});

packages/node/src/integrations/anr/index.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
} from '@sentry/core';
1515
import { NODE_VERSION } from '../../nodeVersion';
1616
import type { NodeClient } from '../../sdk/client';
17+
import { isDebuggerEnabled } from '../../utils/debug';
1718
import type { AnrIntegrationOptions, WorkerStartData } from './common';
1819

1920
const { isPromise } = types;
@@ -98,9 +99,14 @@ const _anrIntegration = ((options: Partial<AnrIntegrationOptions> = {}) => {
9899
});
99100
}
100101
},
101-
setup(initClient: NodeClient) {
102+
async setup(initClient: NodeClient) {
102103
client = initClient;
103104

105+
if (options.captureStackTrace && (await isDebuggerEnabled())) {
106+
logger.warn('ANR captureStackTrace has been disabled because the debugger was already enabled');
107+
options.captureStackTrace = false;
108+
}
109+
104110
// setImmediate is used to ensure that all other integrations have had their setup called first.
105111
// This allows us to call into all integrations to fetch the full context
106112
setImmediate(() => this.startWorker());

packages/node/src/integrations/local-variables/local-variables-async.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { Worker } from 'node:worker_threads';
22
import type { Event, EventHint, Exception, IntegrationFn } from '@sentry/core';
33
import { defineIntegration, logger } from '@sentry/core';
44
import type { NodeClient } from '../../sdk/client';
5+
import { isDebuggerEnabled } from '../../utils/debug';
56
import type { FrameVariables, LocalVariablesIntegrationOptions, LocalVariablesWorkerArgs } from './common';
67
import { LOCAL_VARIABLES_KEY, functionNamesMatch } from './common';
78

@@ -101,13 +102,18 @@ export const localVariablesAsyncIntegration = defineIntegration(((
101102

102103
return {
103104
name: 'LocalVariablesAsync',
104-
setup(client: NodeClient) {
105+
async setup(client: NodeClient) {
105106
const clientOptions = client.getOptions();
106107

107108
if (!clientOptions.includeLocalVariables) {
108109
return;
109110
}
110111

112+
if (await isDebuggerEnabled()) {
113+
logger.warn('Local variables capture has been disabled because the debugger was already enabled');
114+
return;
115+
}
116+
111117
const options: LocalVariablesWorkerArgs = {
112118
...integrationOptions,
113119
debug: logger.isEnabled(),

packages/node/src/integrations/local-variables/local-variables-sync.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { Event, Exception, IntegrationFn, StackFrame, StackParser } from '@
33
import { LRUMap, defineIntegration, getClient, logger } from '@sentry/core';
44
import { NODE_MAJOR } from '../../nodeVersion';
55
import type { NodeClient } from '../../sdk/client';
6+
import { isDebuggerEnabled } from '../../utils/debug';
67
import type {
78
FrameVariables,
89
LocalVariablesIntegrationOptions,
@@ -289,7 +290,7 @@ const _localVariablesSyncIntegration = ((
289290

290291
return {
291292
name: INTEGRATION_NAME,
292-
setupOnce() {
293+
async setupOnce() {
293294
const client = getClient<NodeClient>();
294295
const clientOptions = client?.getOptions();
295296

@@ -306,6 +307,11 @@ const _localVariablesSyncIntegration = ((
306307
return;
307308
}
308309

310+
if (await isDebuggerEnabled()) {
311+
logger.warn('Local variables capture has been disabled because the debugger was already enabled');
312+
return;
313+
}
314+
309315
AsyncSession.create(sessionOverride).then(
310316
session => {
311317
function handlePaused(

packages/node/src/utils/debug.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
let cachedDebuggerEnabled: boolean | undefined;
2+
3+
/**
4+
* Was the debugger enabled when this function was first called?
5+
*/
6+
export async function isDebuggerEnabled(): Promise<boolean> {
7+
if (cachedDebuggerEnabled === undefined) {
8+
try {
9+
// Node can be built without inspector support
10+
const inspector = await import('node:inspector');
11+
cachedDebuggerEnabled = !!inspector.url();
12+
} catch (_) {
13+
cachedDebuggerEnabled = false;
14+
}
15+
}
16+
17+
return cachedDebuggerEnabled;
18+
}

0 commit comments

Comments
 (0)