From ed4a3168a83933b61292acbe78727bb09b9d3145 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Sat, 9 Dec 2023 13:58:46 +0100 Subject: [PATCH 01/19] feat(node): Use worker thread for ANR detection --- .../suites/anr/basic-session.js | 2 +- .../suites/anr/basic.js | 2 +- .../suites/anr/basic.mjs | 2 +- .../suites/anr/forked.js | 2 +- .../node-integration-tests/suites/anr/test.ts | 44 +-- packages/node/src/anr/debugger.ts | 55 --- packages/node/src/anr/index.ts | 173 ++++++--- packages/node/src/anr/websocket.ts | 366 ------------------ packages/node/src/sdk.ts | 4 +- 9 files changed, 137 insertions(+), 513 deletions(-) delete mode 100644 packages/node/src/anr/debugger.ts delete mode 100644 packages/node/src/anr/websocket.ts diff --git a/packages/node-integration-tests/suites/anr/basic-session.js b/packages/node-integration-tests/suites/anr/basic-session.js index 29cdc17e76c9..afc9b35c6bc8 100644 --- a/packages/node-integration-tests/suites/anr/basic-session.js +++ b/packages/node-integration-tests/suites/anr/basic-session.js @@ -7,7 +7,7 @@ const { transport } = require('./test-transport.js'); // close both processes after 5 seconds setTimeout(() => { process.exit(); -}, 5000); +}, 10000); Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', diff --git a/packages/node-integration-tests/suites/anr/basic.js b/packages/node-integration-tests/suites/anr/basic.js index 33c4151a19f1..47b2326410cb 100644 --- a/packages/node-integration-tests/suites/anr/basic.js +++ b/packages/node-integration-tests/suites/anr/basic.js @@ -7,7 +7,7 @@ const { transport } = require('./test-transport.js'); // close both processes after 5 seconds setTimeout(() => { process.exit(); -}, 5000); +}, 10000); Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', diff --git a/packages/node-integration-tests/suites/anr/basic.mjs b/packages/node-integration-tests/suites/anr/basic.mjs index 3d10dc556076..cd858c4d1b4d 100644 --- a/packages/node-integration-tests/suites/anr/basic.mjs +++ b/packages/node-integration-tests/suites/anr/basic.mjs @@ -7,7 +7,7 @@ const { transport } = await import('./test-transport.js'); // close both processes after 5 seconds setTimeout(() => { process.exit(); -}, 5000); +}, 10000); Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', diff --git a/packages/node-integration-tests/suites/anr/forked.js b/packages/node-integration-tests/suites/anr/forked.js index 33c4151a19f1..47b2326410cb 100644 --- a/packages/node-integration-tests/suites/anr/forked.js +++ b/packages/node-integration-tests/suites/anr/forked.js @@ -7,7 +7,7 @@ const { transport } = require('./test-transport.js'); // close both processes after 5 seconds setTimeout(() => { process.exit(); -}, 5000); +}, 10000); Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', diff --git a/packages/node-integration-tests/suites/anr/test.ts b/packages/node-integration-tests/suites/anr/test.ts index 0c815c280f00..6f611dca31ab 100644 --- a/packages/node-integration-tests/suites/anr/test.ts +++ b/packages/node-integration-tests/suites/anr/test.ts @@ -26,10 +26,12 @@ function parseJsonLines(input: string, expected: number): T describe('should report ANR when event loop blocked', () => { test('CJS', done => { - // The stack trace is different when node < 12 - const testFramesDetails = NODE_VERSION >= 12; + if (NODE_VERSION < 12) { + done(); + return; + } - expect.assertions(testFramesDetails ? 7 : 5); + expect.assertions(7); const testScriptPath = path.resolve(__dirname, 'basic.js'); @@ -41,10 +43,8 @@ describe('should report ANR when event loop blocked', () => { expect(event.exception?.values?.[0].value).toEqual('Application Not Responding for at least 200 ms'); expect(event.exception?.values?.[0].stacktrace?.frames?.length).toBeGreaterThan(4); - if (testFramesDetails) { - expect(event.exception?.values?.[0].stacktrace?.frames?.[2].function).toEqual('?'); - expect(event.exception?.values?.[0].stacktrace?.frames?.[3].function).toEqual('longWork'); - } + expect(event.exception?.values?.[0].stacktrace?.frames?.[2].function).toEqual('?'); + expect(event.exception?.values?.[0].stacktrace?.frames?.[3].function).toEqual('longWork'); done(); }); @@ -66,7 +66,7 @@ describe('should report ANR when event loop blocked', () => { expect(event.exception?.values?.[0].mechanism).toEqual({ type: 'ANR' }); expect(event.exception?.values?.[0].type).toEqual('ApplicationNotResponding'); expect(event.exception?.values?.[0].value).toEqual('Application Not Responding for at least 200 ms'); - expect(event.exception?.values?.[0].stacktrace?.frames?.length).toBeGreaterThan(4); + expect(event.exception?.values?.[0].stacktrace?.frames?.length).toBeGreaterThanOrEqual(4); expect(event.exception?.values?.[0].stacktrace?.frames?.[2].function).toEqual('?'); expect(event.exception?.values?.[0].stacktrace?.frames?.[3].function).toEqual('longWork'); @@ -75,10 +75,12 @@ describe('should report ANR when event loop blocked', () => { }); test('With session', done => { - // The stack trace is different when node < 12 - const testFramesDetails = NODE_VERSION >= 12; + if (NODE_VERSION < 12) { + done(); + return; + } - expect.assertions(testFramesDetails ? 9 : 7); + expect.assertions(9); const testScriptPath = path.resolve(__dirname, 'basic-session.js'); @@ -90,10 +92,8 @@ describe('should report ANR when event loop blocked', () => { expect(event.exception?.values?.[0].value).toEqual('Application Not Responding for at least 200 ms'); expect(event.exception?.values?.[0].stacktrace?.frames?.length).toBeGreaterThan(4); - if (testFramesDetails) { - expect(event.exception?.values?.[0].stacktrace?.frames?.[2].function).toEqual('?'); - expect(event.exception?.values?.[0].stacktrace?.frames?.[3].function).toEqual('longWork'); - } + expect(event.exception?.values?.[0].stacktrace?.frames?.[2].function).toEqual('?'); + expect(event.exception?.values?.[0].stacktrace?.frames?.[3].function).toEqual('longWork'); expect(session.status).toEqual('abnormal'); expect(session.abnormal_mechanism).toEqual('anr_foreground'); @@ -103,10 +103,12 @@ describe('should report ANR when event loop blocked', () => { }); test('from forked process', done => { - // The stack trace is different when node < 12 - const testFramesDetails = NODE_VERSION >= 12; + if (NODE_VERSION < 12) { + done(); + return; + } - expect.assertions(testFramesDetails ? 7 : 5); + expect.assertions(7); const testScriptPath = path.resolve(__dirname, 'forker.js'); @@ -118,10 +120,8 @@ describe('should report ANR when event loop blocked', () => { expect(event.exception?.values?.[0].value).toEqual('Application Not Responding for at least 200 ms'); expect(event.exception?.values?.[0].stacktrace?.frames?.length).toBeGreaterThan(4); - if (testFramesDetails) { - expect(event.exception?.values?.[0].stacktrace?.frames?.[2].function).toEqual('?'); - expect(event.exception?.values?.[0].stacktrace?.frames?.[3].function).toEqual('longWork'); - } + expect(event.exception?.values?.[0].stacktrace?.frames?.[2].function).toEqual('?'); + expect(event.exception?.values?.[0].stacktrace?.frames?.[3].function).toEqual('longWork'); done(); }); diff --git a/packages/node/src/anr/debugger.ts b/packages/node/src/anr/debugger.ts deleted file mode 100644 index 01b2a90fe0f9..000000000000 --- a/packages/node/src/anr/debugger.ts +++ /dev/null @@ -1,55 +0,0 @@ -import type { StackFrame } from '@sentry/types'; -import { createDebugPauseMessageHandler } from '@sentry/utils'; -import type { Debugger } from 'inspector'; - -import { getModuleFromFilename } from '../module'; -import { createWebSocketClient } from './websocket'; - -// The only messages we care about -type DebugMessage = - | { - method: 'Debugger.scriptParsed'; - params: Debugger.ScriptParsedEventDataType; - } - | { method: 'Debugger.paused'; params: Debugger.PausedEventDataType }; - -/** - * Wraps a websocket connection with the basic logic of the Node debugger protocol. - * @param url The URL to connect to - * @param onMessage A callback that will be called with each return message from the debugger - * @returns A function that can be used to send commands to the debugger - */ -async function webSocketDebugger( - url: string, - onMessage: (message: DebugMessage) => void, -): Promise<(method: string) => void> { - let id = 0; - const webSocket = await createWebSocketClient(url); - - webSocket.on('message', (data: Buffer) => { - const message = JSON.parse(data.toString()) as DebugMessage; - onMessage(message); - }); - - return (method: string) => { - webSocket.send(JSON.stringify({ id: id++, method })); - }; -} - -/** - * Captures stack traces from the Node debugger. - * @param url The URL to connect to - * @param callback A callback that will be called with the stack frames - * @returns A function that triggers the debugger to pause and capture a stack trace - */ -export async function captureStackTrace(url: string, callback: (frames: StackFrame[]) => void): Promise<() => void> { - const sendCommand: (method: string) => void = await webSocketDebugger( - url, - createDebugPauseMessageHandler(cmd => sendCommand(cmd), getModuleFromFilename, callback), - ); - - return () => { - sendCommand('Debugger.enable'); - sendCommand('Debugger.pause'); - }; -} diff --git a/packages/node/src/anr/index.ts b/packages/node/src/anr/index.ts index 84a3ebe52920..1992a3845b1e 100644 --- a/packages/node/src/anr/index.ts +++ b/packages/node/src/anr/index.ts @@ -1,23 +1,24 @@ -import { spawn } from 'child_process'; import { getClient, makeSession, updateSession } from '@sentry/core'; import type { Event, Session, StackFrame } from '@sentry/types'; -import { logger, watchdogTimer } from '@sentry/utils'; +import { createDebugPauseMessageHandler, dynamicRequire, logger, watchdogTimer } from '@sentry/utils'; +import type { Session as InspectorSession } from 'inspector'; -import { addEventProcessor, captureEvent, flush, getCurrentHub } from '..'; -import { captureStackTrace } from './debugger'; +import type { MessagePort, Worker } from 'worker_threads'; +import { addEventProcessor, captureEvent, flush, getCurrentHub, getModuleFromFilename } from '..'; +import { NODE_VERSION } from '../nodeVersion'; const DEFAULT_INTERVAL = 50; const DEFAULT_HANG_THRESHOLD = 5000; interface Options { /** - * The app entry script. This is used to run the same script as the child process. + * The app entry script. This is used to run the same script as the ANR worker. * * Defaults to `process.argv[1]`. */ entryScript: string; /** - * Interval to send heartbeat messages to the child process. + * Interval to send heartbeat messages to the ANR worker. * * Defaults to 50ms. */ @@ -61,6 +62,23 @@ function createAnrEvent(blockedMs: number, frames?: StackFrame[]): Event { }; } +type WorkerThreads = { + Worker: typeof Worker; + isMainThread: boolean; + parentPort: null | MessagePort; + workerData: { inspectorUrl?: string }; +}; + +/** + * We need to use dynamicRequire because worker_threads is not available in node < v12 and webpack error will when + * targeting those versions + */ +function getWorkerThreads(): WorkerThreads { + return dynamicRequire(module, 'worker_threads'); +} + +type InspectorSessionNodeV12 = InspectorSession & { connectToMainThread: () => void }; + interface InspectorApi { open: (port: number) => void; url: () => string | undefined; @@ -86,7 +104,9 @@ function startInspector(startPort: number = 9229): string | undefined { return inspectorUrl; } -function startChildProcess(options: Options): void { +function startAnrWorker(options: Options): void { + const { Worker } = getWorkerThreads(); + function log(message: string, ...args: unknown[]): void { logger.log(`[ANR] ${message}`, ...args); } @@ -94,38 +114,29 @@ function startChildProcess(options: Options): void { const hub = getCurrentHub(); try { - const env = { ...process.env }; - env.SENTRY_ANR_CHILD_PROCESS = 'true'; + log(`Spawning worker with entryScript:'${options.entryScript}'`); - if (options.captureStackTrace) { - env.SENTRY_INSPECT_URL = startInspector(); - } - - log(`Spawning child process with execPath:'${process.execPath}' and entryScript:'${options.entryScript}'`); - - const child = spawn(process.execPath, [options.entryScript], { - env, - stdio: logger.isEnabled() ? ['inherit', 'inherit', 'inherit', 'ipc'] : ['ignore', 'ignore', 'ignore', 'ipc'], - }); - // The child process should not keep the main process alive - child.unref(); + const inspectorUrl = options.captureStackTrace ? startInspector() : undefined; + const worker = new Worker(options.entryScript, { workerData: { inspectorUrl } }); + // The worker should not keep the main process alive + worker.unref(); const timer = setInterval(() => { try { const currentSession = hub.getScope()?.getSession(); - // We need to copy the session object and remove the toJSON method so it can be sent to the child process + // We need to copy the session object and remove the toJSON method so it can be sent to the worker // serialized without making it a SerializedSession const session = currentSession ? { ...currentSession, toJSON: undefined } : undefined; - // message the child process to tell it the main event loop is still running - child.send({ session }); + // message the worker to tell it the main event loop is still running + worker.postMessage({ session }); } catch (_) { // } }, options.pollInterval); - child.on('message', (msg: string) => { + worker.on('message', (msg: string) => { if (msg === 'session-ended') { - log('ANR event sent from child process. Clearing session in this process.'); + log('ANR event sent from ANR worker. Clearing session in this process.'); hub.getScope()?.setSession(undefined); } }); @@ -133,15 +144,14 @@ function startChildProcess(options: Options): void { const end = (type: string): ((...args: unknown[]) => void) => { return (...args): void => { clearInterval(timer); - log(`Child process ${type}`, ...args); + log(`ANR worker ${type}`, ...args); }; }; - child.on('error', end('error')); - child.on('disconnect', end('disconnect')); - child.on('exit', end('exit')); + worker.on('error', end('error')); + worker.on('exit', end('exit')); } catch (e) { - log('Failed to start child process', e); + log('Failed to start worker', e); } } @@ -159,17 +169,27 @@ function createHrTimer(): { getTimeMs: () => number; reset: () => void } { }; } -function handleChildProcess(options: Options): void { - process.title = 'sentry-anr'; +function handlerAnrWorker(options: Options): void { + const { parentPort, workerData } = getWorkerThreads(); + + let anrEventSent = false; function log(message: string): void { - logger.log(`[ANR child process] ${message}`); + logger.log(`[ANR Worker] ${message}`); } log('Started'); let session: Session | undefined; function sendAnrEvent(frames?: StackFrame[]): void { + if (anrEventSent) { + return; + } + + anrEventSent = true; + + log('Sending ANR event'); + if (session) { log('Sending abnormal session'); updateSession(session, { status: 'abnormal', abnormal_mechanism: 'anr_foreground' }); @@ -177,7 +197,7 @@ function handleChildProcess(options: Options): void { try { // Notify the main process that the session has ended so the session can be cleared from the scope - process.send?.('session-ended'); + parentPort?.postMessage('session-ended'); } catch (_) { // ignore } @@ -186,13 +206,16 @@ function handleChildProcess(options: Options): void { captureEvent(createAnrEvent(options.anrThreshold, frames)); void flush(3000).then(() => { - // We only capture one event to avoid spamming users with errors - process.exit(); + // We exit so we only capture one event to avoid spamming users with errors + // We wait 5 seconds to ensure stdio has been flushed from the worker + setTimeout(() => { + process.exit(); + }, 5_000); }); } addEventProcessor(event => { - // Strip sdkProcessingMetadata from all child process events to remove trace info + // Strip sdkProcessingMetadata from all ANR worker events to remove trace info delete event.sdkProcessingMetadata; event.tags = { ...event.tags, @@ -201,27 +224,43 @@ function handleChildProcess(options: Options): void { return event; }); - let debuggerPause: Promise<() => void> | undefined; + let debuggerPause: () => void | undefined; - // if attachStackTrace is enabled, we'll have a debugger url to connect to - if (process.env.SENTRY_INSPECT_URL) { - log('Connecting to debugger'); + const { inspectorUrl } = workerData; - debuggerPause = captureStackTrace(process.env.SENTRY_INSPECT_URL, frames => { - log('Capturing event with stack frames'); - sendAnrEvent(frames); + // if attachStackTrace was enabled, we'll have a debugger url to connect to + if (inspectorUrl) { + // eslint-disable-next-line @typescript-eslint/no-var-requires + const { Session } = require('inspector'); + log('Connecting to debugger'); + const session: InspectorSessionNodeV12 = new Session(); + session.connectToMainThread(); + + const handler = createDebugPauseMessageHandler( + cmd => session.post(cmd), + getModuleFromFilename, + frames => sendAnrEvent(frames), + ); + + session.on('inspectorNotification', params => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + handler(params as any); }); + + debuggerPause = () => { + session.post('Debugger.enable', () => { + session.post('Debugger.pause'); + }); + }; } async function watchdogTimeout(): Promise { log('Watchdog timeout'); try { - const pauseAndCapture = await debuggerPause; - - if (pauseAndCapture) { + if (debuggerPause) { log('Pausing debugger to capture stack trace'); - pauseAndCapture(); + debuggerPause(); return; } } catch (_) { @@ -234,32 +273,34 @@ function handleChildProcess(options: Options): void { const { poll } = watchdogTimer(createHrTimer, options.pollInterval, options.anrThreshold, watchdogTimeout); - process.on('message', (msg: { session: Session | undefined }) => { + parentPort?.on('message', (msg: { session: Session | undefined }) => { if (msg.session) { session = makeSession(msg.session); } + poll(); }); - process.on('disconnect', () => { - // Parent process has exited. - process.exit(); - }); } /** - * Returns true if the current process is an ANR child process. + * Returns true if the current thread is the ANR worker. */ -export function isAnrChildProcess(): boolean { - return !!process.send && !!process.env.SENTRY_ANR_CHILD_PROCESS; +export function isAnrWorker(): boolean { + try { + const { isMainThread } = dynamicRequire(module, 'worker_threads') as WorkerThreads; + return !isMainThread; + } catch (_) { + return false; + } } /** * **Note** This feature is still in beta so there may be breaking changes in future releases. * - * Starts a child process that detects Application Not Responding (ANR) errors. + * Starts a ANR worker that detects Application Not Responding (ANR) errors. * - * It's important to await on the returned promise before your app code to ensure this code does not run in the ANR - * child process. + * It's important to await on the returned promise before your app code to ensure this code does not run in the + * ANR worker. * * ```js * import { init, enableAnrDetection } from '@sentry/node'; @@ -277,6 +318,10 @@ export function isAnrChildProcess(): boolean { * ``` */ export function enableAnrDetection(options: Partial): Promise { + if ((NODE_VERSION.major || 0) < 12 || ((NODE_VERSION.major || 0) === 12 && (NODE_VERSION.minor || 0) < 11)) { + throw new Error('ANR detection requires Node 12.11.0 or later'); + } + // When pm2 runs the script in cluster mode, process.argv[1] is the pm2 script and process.env.pm_exec_path is the // path to the entry script const entryScript = options.entryScript || process.env.pm_exec_path || process.argv[1]; @@ -290,14 +335,14 @@ export function enableAnrDetection(options: Partial): Promise { debug: !!options.debug, }; - if (isAnrChildProcess()) { - handleChildProcess(anrOptions); - // In the child process, the promise never resolves which stops the app code from running + if (isAnrWorker()) { + handlerAnrWorker(anrOptions); + // In the ANR worker, the promise never resolves which stops the app code from running return new Promise(() => { // Never resolve }); } else { - startChildProcess(anrOptions); + startAnrWorker(anrOptions); // In the main process, the promise resolves immediately return Promise.resolve(); } diff --git a/packages/node/src/anr/websocket.ts b/packages/node/src/anr/websocket.ts deleted file mode 100644 index 7229f0fc07e7..000000000000 --- a/packages/node/src/anr/websocket.ts +++ /dev/null @@ -1,366 +0,0 @@ -/* eslint-disable no-bitwise */ -/** - * A simple WebSocket client implementation copied from Rome before being modified for our use: - * https://github.com/jeremyBanks/rome/tree/b034dd22d5f024f87c50eef2872e22b3ad48973a/packages/%40romejs/codec-websocket - * - * Original license: - * - * MIT License - * - * Copyright (c) Facebook, Inc. and its affiliates. - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in all - * copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. - */ - -import * as crypto from 'crypto'; -import { EventEmitter } from 'events'; -import * as http from 'http'; -import type { Socket } from 'net'; -import * as url from 'url'; - -type BuildFrameOpts = { - opcode: number; - fin: boolean; - data: Buffer; -}; - -type Frame = { - fin: boolean; - opcode: number; - mask: undefined | Buffer; - payload: Buffer; - payloadLength: number; -}; - -const OPCODES = { - CONTINUATION: 0, - TEXT: 1, - BINARY: 2, - TERMINATE: 8, - PING: 9, - PONG: 10, -}; - -const GUID = '258EAFA5-E914-47DA-95CA-C5AB0DC85B11'; - -function isCompleteFrame(frame: Frame): boolean { - return Buffer.byteLength(frame.payload) >= frame.payloadLength; -} - -function unmaskPayload(payload: Buffer, mask: undefined | Buffer, offset: number): Buffer { - if (mask === undefined) { - return payload; - } - - for (let i = 0; i < payload.length; i++) { - payload[i] ^= mask[(offset + i) & 3]; - } - - return payload; -} - -function buildFrame(opts: BuildFrameOpts): Buffer { - const { opcode, fin, data } = opts; - - let offset = 6; - let dataLength = data.length; - - if (dataLength >= 65_536) { - offset += 8; - dataLength = 127; - } else if (dataLength > 125) { - offset += 2; - dataLength = 126; - } - - const head = Buffer.allocUnsafe(offset); - - head[0] = fin ? opcode | 128 : opcode; - head[1] = dataLength; - - if (dataLength === 126) { - head.writeUInt16BE(data.length, 2); - } else if (dataLength === 127) { - head.writeUInt32BE(0, 2); - head.writeUInt32BE(data.length, 6); - } - - const mask = crypto.randomBytes(4); - head[1] |= 128; - head[offset - 4] = mask[0]; - head[offset - 3] = mask[1]; - head[offset - 2] = mask[2]; - head[offset - 1] = mask[3]; - - const masked = Buffer.alloc(dataLength); - for (let i = 0; i < dataLength; ++i) { - masked[i] = data[i] ^ mask[i & 3]; - } - - return Buffer.concat([head, masked]); -} - -function parseFrame(buffer: Buffer): Frame { - const firstByte = buffer.readUInt8(0); - const isFinalFrame: boolean = Boolean((firstByte >>> 7) & 1); - const opcode: number = firstByte & 15; - - const secondByte: number = buffer.readUInt8(1); - const isMasked: boolean = Boolean((secondByte >>> 7) & 1); - - // Keep track of our current position as we advance through the buffer - let currentOffset = 2; - let payloadLength = secondByte & 127; - if (payloadLength > 125) { - if (payloadLength === 126) { - payloadLength = buffer.readUInt16BE(currentOffset); - currentOffset += 2; - } else if (payloadLength === 127) { - const leftPart = buffer.readUInt32BE(currentOffset); - currentOffset += 4; - - // The maximum safe integer in JavaScript is 2^53 - 1. An error is returned - - // if payload length is greater than this number. - if (leftPart >= Number.MAX_SAFE_INTEGER) { - throw new Error('Unsupported WebSocket frame: payload length > 2^53 - 1'); - } - - const rightPart = buffer.readUInt32BE(currentOffset); - currentOffset += 4; - - payloadLength = leftPart * Math.pow(2, 32) + rightPart; - } else { - throw new Error('Unknown payload length'); - } - } - - // Get the masking key if one exists - let mask; - if (isMasked) { - mask = buffer.slice(currentOffset, currentOffset + 4); - currentOffset += 4; - } - - const payload = unmaskPayload(buffer.slice(currentOffset), mask, 0); - - return { - fin: isFinalFrame, - opcode, - mask, - payload, - payloadLength, - }; -} - -function createKey(key: string): string { - return crypto.createHash('sha1').update(`${key}${GUID}`).digest('base64'); -} - -class WebSocketInterface extends EventEmitter { - private _alive: boolean; - private _incompleteFrame: undefined | Frame; - private _unfinishedFrame: undefined | Frame; - private _socket: Socket; - - public constructor(socket: Socket) { - super(); - // When a frame is set here then any additional continuation frames payloads will be appended - this._unfinishedFrame = undefined; - - // When a frame is set here, all additional chunks will be appended until we reach the correct payloadLength - this._incompleteFrame = undefined; - - this._socket = socket; - this._alive = true; - - socket.on('data', buff => { - this._addBuffer(buff); - }); - - socket.on('error', (err: NodeJS.ErrnoException) => { - if (err.code === 'ECONNRESET') { - this.emit('close'); - } else { - this.emit('error'); - } - }); - - socket.on('close', () => { - this.end(); - }); - } - - public end(): void { - if (!this._alive) { - return; - } - - this._alive = false; - this.emit('close'); - this._socket.end(); - } - - public send(buff: string): void { - this._sendFrame({ - opcode: OPCODES.TEXT, - fin: true, - data: Buffer.from(buff), - }); - } - - private _sendFrame(frameOpts: BuildFrameOpts): void { - this._socket.write(buildFrame(frameOpts)); - } - - private _completeFrame(frame: Frame): void { - // If we have an unfinished frame then only allow continuations - const { _unfinishedFrame: unfinishedFrame } = this; - if (unfinishedFrame !== undefined) { - if (frame.opcode === OPCODES.CONTINUATION) { - unfinishedFrame.payload = Buffer.concat([ - unfinishedFrame.payload, - unmaskPayload(frame.payload, unfinishedFrame.mask, unfinishedFrame.payload.length), - ]); - - if (frame.fin) { - this._unfinishedFrame = undefined; - this._completeFrame(unfinishedFrame); - } - return; - } else { - // Silently ignore the previous frame... - this._unfinishedFrame = undefined; - } - } - - if (frame.fin) { - if (frame.opcode === OPCODES.PING) { - this._sendFrame({ - opcode: OPCODES.PONG, - fin: true, - data: frame.payload, - }); - } else { - // Trim off any excess payload - let excess; - if (frame.payload.length > frame.payloadLength) { - excess = frame.payload.slice(frame.payloadLength); - frame.payload = frame.payload.slice(0, frame.payloadLength); - } - - this.emit('message', frame.payload); - - if (excess !== undefined) { - this._addBuffer(excess); - } - } - } else { - this._unfinishedFrame = frame; - } - } - - private _addBufferToIncompleteFrame(incompleteFrame: Frame, buff: Buffer): void { - incompleteFrame.payload = Buffer.concat([ - incompleteFrame.payload, - unmaskPayload(buff, incompleteFrame.mask, incompleteFrame.payload.length), - ]); - - if (isCompleteFrame(incompleteFrame)) { - this._incompleteFrame = undefined; - this._completeFrame(incompleteFrame); - } - } - - private _addBuffer(buff: Buffer): void { - // Check if we're still waiting for the rest of a payload - const { _incompleteFrame: incompleteFrame } = this; - if (incompleteFrame !== undefined) { - this._addBufferToIncompleteFrame(incompleteFrame, buff); - return; - } - - // There needs to be atleast two values in the buffer for us to parse - // a frame from it. - // See: https://github.com/getsentry/sentry-javascript/issues/9307 - if (buff.length <= 1) { - return; - } - - const frame = parseFrame(buff); - - if (isCompleteFrame(frame)) { - // Frame has been completed! - this._completeFrame(frame); - } else { - this._incompleteFrame = frame; - } - } -} - -/** - * Creates a WebSocket client - */ -export async function createWebSocketClient(rawUrl: string): Promise { - const parts = url.parse(rawUrl); - - return new Promise((resolve, reject) => { - const key = crypto.randomBytes(16).toString('base64'); - const digest = createKey(key); - - const req = http.request({ - hostname: parts.hostname, - port: parts.port, - path: parts.path, - method: 'GET', - headers: { - Connection: 'Upgrade', - Upgrade: 'websocket', - 'Sec-WebSocket-Key': key, - 'Sec-WebSocket-Version': '13', - }, - }); - - req.on('response', (res: http.IncomingMessage) => { - if (res.statusCode && res.statusCode >= 400) { - process.stderr.write(`Unexpected HTTP code: ${res.statusCode}\n`); - res.pipe(process.stderr); - } else { - res.pipe(process.stderr); - } - }); - - req.on('upgrade', (res: http.IncomingMessage, socket: Socket) => { - if (res.headers['sec-websocket-accept'] !== digest) { - socket.end(); - reject(new Error(`Digest mismatch ${digest} !== ${res.headers['sec-websocket-accept']}`)); - return; - } - - const client = new WebSocketInterface(socket); - resolve(client); - }); - - req.on('error', err => { - reject(err); - }); - - req.end(); - }); -} diff --git a/packages/node/src/sdk.ts b/packages/node/src/sdk.ts index 5ef42128d9ab..d8498ef7ecde 100644 --- a/packages/node/src/sdk.ts +++ b/packages/node/src/sdk.ts @@ -15,7 +15,7 @@ import { tracingContextFromHeaders, } from '@sentry/utils'; -import { isAnrChildProcess } from './anr'; +import { isAnrWorker } from './anr'; import { setNodeAsyncContextStrategy } from './async'; import { NodeClient } from './client'; import { @@ -112,7 +112,7 @@ export const defaultIntegrations = [ */ // eslint-disable-next-line complexity export function init(options: NodeOptions = {}): void { - if (isAnrChildProcess()) { + if (isAnrWorker()) { options.autoSessionTracking = false; options.tracesSampleRate = 0; } From 14fe3a502c141c57652f2a534959641fcfdd43e9 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 11 Dec 2023 14:35:38 +0100 Subject: [PATCH 02/19] Remove utility --- .../node-integration-tests/suites/anr/test.ts | 5 +- packages/node/src/anr/index.ts | 88 ++++++++++++++----- packages/utils/src/anr.ts | 41 +-------- 3 files changed, 71 insertions(+), 63 deletions(-) diff --git a/packages/node-integration-tests/suites/anr/test.ts b/packages/node-integration-tests/suites/anr/test.ts index 6f611dca31ab..de4032479f03 100644 --- a/packages/node-integration-tests/suites/anr/test.ts +++ b/packages/node-integration-tests/suites/anr/test.ts @@ -31,7 +31,7 @@ describe('should report ANR when event loop blocked', () => { return; } - expect.assertions(7); + expect.assertions(9); const testScriptPath = path.resolve(__dirname, 'basic.js'); @@ -46,6 +46,9 @@ describe('should report ANR when event loop blocked', () => { expect(event.exception?.values?.[0].stacktrace?.frames?.[2].function).toEqual('?'); expect(event.exception?.values?.[0].stacktrace?.frames?.[3].function).toEqual('longWork'); + expect(event.contexts?.trace?.trace_id).toBeDefined(); + expect(event.contexts?.trace?.span_id).toBeDefined(); + done(); }); }); diff --git a/packages/node/src/anr/index.ts b/packages/node/src/anr/index.ts index 1992a3845b1e..7005b73ac4de 100644 --- a/packages/node/src/anr/index.ts +++ b/packages/node/src/anr/index.ts @@ -1,6 +1,12 @@ import { getClient, makeSession, updateSession } from '@sentry/core'; -import type { Event, Session, StackFrame } from '@sentry/types'; -import { createDebugPauseMessageHandler, dynamicRequire, logger, watchdogTimer } from '@sentry/utils'; +import type { Event, Session, StackFrame, TraceContext } from '@sentry/types'; +import { + callFrameToStackFrame, + dynamicRequire, + logger, + stripSentryFramesAndReverse, + watchdogTimer, +} from '@sentry/utils'; import type { Session as InspectorSession } from 'inspector'; import type { MessagePort, Worker } from 'worker_threads'; @@ -136,20 +142,20 @@ function startAnrWorker(options: Options): void { worker.on('message', (msg: string) => { if (msg === 'session-ended') { - log('ANR event sent from ANR worker. Clearing session in this process.'); + log('ANR event sent from ANR worker. Clearing session in this thread.'); hub.getScope()?.setSession(undefined); } }); - const end = (type: string): ((...args: unknown[]) => void) => { - return (...args): void => { - clearInterval(timer); - log(`ANR worker ${type}`, ...args); - }; - }; + worker.on('error', (err: Error) => { + clearInterval(timer); + log('ANR worker error', err); + }); - worker.on('error', end('error')); - worker.on('exit', end('exit')); + worker.on('exit', (code: number) => { + clearInterval(timer); + log('ANR worker exit', code); + }); } catch (e) { log('Failed to start worker', e); } @@ -181,7 +187,7 @@ function handlerAnrWorker(options: Options): void { log('Started'); let session: Session | undefined; - function sendAnrEvent(frames?: StackFrame[]): void { + function sendAnrEvent(frames?: StackFrame[], traceContext?: TraceContext): void { if (anrEventSent) { return; } @@ -203,7 +209,9 @@ function handlerAnrWorker(options: Options): void { } } - captureEvent(createAnrEvent(options.anrThreshold, frames)); + captureEvent(createAnrEvent(options.anrThreshold, frames), { + captureContext: { contexts: { trace: traceContext } }, + }); void flush(3000).then(() => { // We exit so we only capture one event to avoid spamming users with errors @@ -230,21 +238,55 @@ function handlerAnrWorker(options: Options): void { // if attachStackTrace was enabled, we'll have a debugger url to connect to if (inspectorUrl) { + log('Connecting to debugger'); + // eslint-disable-next-line @typescript-eslint/no-var-requires const { Session } = require('inspector'); - log('Connecting to debugger'); const session: InspectorSessionNodeV12 = new Session(); session.connectToMainThread(); - const handler = createDebugPauseMessageHandler( - cmd => session.post(cmd), - getModuleFromFilename, - frames => sendAnrEvent(frames), - ); + // Collect scriptId -> url map so we can look up the filenames later + const scripts = new Map(); - session.on('inspectorNotification', params => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - handler(params as any); + session.on('Debugger.scriptParsed', event => { + scripts.set(event.params.scriptId, event.params.url); + }); + + session.on('Debugger.paused', event => { + if (event.params.reason !== 'other') { + return; + } + + // copy the frames + const callFrames = [...event.params.callFrames]; + + const stackFrames = stripSentryFramesAndReverse( + callFrames.map(frame => + callFrameToStackFrame(frame, scripts.get(frame.location.scriptId), getModuleFromFilename), + ), + ); + + // Evaluate a script in the currently paused context + session.post( + 'Runtime.evaluate', + { + // Grab the trace context from the current scope + expression: 'const ctx = __SENTRY__.hub.getScope().getPropagationContext(); ctx.traceId + "-" + ctx.spanId', + // Don't re-trigger the debugger if this causes an error + silent: true, + }, + (_, param) => { + const traceId = param && param.result ? (param.result.value as string) : '-'; + const [trace_id, span_id] = traceId.split('-'); + + // Resume immediately + session.post('Debugger.resume'); + session.post('Debugger.disable'); + + const context = trace_id && span_id ? { trace_id, span_id } : undefined; + sendAnrEvent(stackFrames, context); + }, + ); }); debuggerPause = () => { @@ -267,7 +309,7 @@ function handlerAnrWorker(options: Options): void { // ignore } - log('Capturing event'); + log('Capturing event without a stack trace'); sendAnrEvent(); } diff --git a/packages/utils/src/anr.ts b/packages/utils/src/anr.ts index 89990c3414f7..5312d5632ac9 100644 --- a/packages/utils/src/anr.ts +++ b/packages/utils/src/anr.ts @@ -1,7 +1,7 @@ import type { StackFrame } from '@sentry/types'; import { dropUndefinedKeys } from './object'; -import { filenameIsInApp, stripSentryFramesAndReverse } from './stacktrace'; +import { filenameIsInApp } from './stacktrace'; type WatchdogReturn = { /** Resets the watchdog timer */ @@ -80,7 +80,7 @@ interface PausedEventDataType { /** * Converts Debugger.CallFrame to Sentry StackFrame */ -function callFrameToStackFrame( +export function callFrameToStackFrame( frame: CallFrame, url: string | undefined, getModuleFromFilename: (filename: string | undefined) => string | undefined, @@ -100,40 +100,3 @@ function callFrameToStackFrame( in_app: filename ? filenameIsInApp(filename) : undefined, }); } - -// The only messages we care about -type DebugMessage = - | { method: 'Debugger.scriptParsed'; params: ScriptParsedEventDataType } - | { method: 'Debugger.paused'; params: PausedEventDataType }; - -/** - * Creates a message handler from the v8 debugger protocol and passed stack frames to the callback when paused. - */ -export function createDebugPauseMessageHandler( - sendCommand: (message: string) => void, - getModuleFromFilename: (filename?: string) => string | undefined, - pausedStackFrames: (frames: StackFrame[]) => void, -): (message: DebugMessage) => void { - // Collect scriptId -> url map so we can look up the filenames later - const scripts = new Map(); - - return message => { - if (message.method === 'Debugger.scriptParsed') { - scripts.set(message.params.scriptId, message.params.url); - } else if (message.method === 'Debugger.paused') { - // copy the frames - const callFrames = [...message.params.callFrames]; - // and resume immediately - sendCommand('Debugger.resume'); - sendCommand('Debugger.disable'); - - const stackFrames = stripSentryFramesAndReverse( - callFrames.map(frame => - callFrameToStackFrame(frame, scripts.get(frame.location.scriptId), getModuleFromFilename), - ), - ); - - pausedStackFrames(stackFrames); - } - }; -} From e5c82dd662b1460a559ac989dd82594b55a38092 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 12 Dec 2023 16:00:40 +0100 Subject: [PATCH 03/19] PR feedback --- packages/node/src/anr/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/node/src/anr/index.ts b/packages/node/src/anr/index.ts index 7005b73ac4de..76d3e8296d5b 100644 --- a/packages/node/src/anr/index.ts +++ b/packages/node/src/anr/index.ts @@ -147,12 +147,12 @@ function startAnrWorker(options: Options): void { } }); - worker.on('error', (err: Error) => { + worker.once('error', (err: Error) => { clearInterval(timer); log('ANR worker error', err); }); - worker.on('exit', (code: number) => { + worker.once('exit', (code: number) => { clearInterval(timer); log('ANR worker exit', code); }); From dce36842f10a79275ddf9ed38bd13e8e1549676e Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 12 Dec 2023 16:35:20 +0100 Subject: [PATCH 04/19] remove outdated comments --- packages/node-integration-tests/suites/anr/basic-session.js | 1 - packages/node-integration-tests/suites/anr/basic.js | 1 - packages/node-integration-tests/suites/anr/basic.mjs | 1 - packages/node-integration-tests/suites/anr/forked.js | 1 - 4 files changed, 4 deletions(-) diff --git a/packages/node-integration-tests/suites/anr/basic-session.js b/packages/node-integration-tests/suites/anr/basic-session.js index afc9b35c6bc8..cfb2b7f05190 100644 --- a/packages/node-integration-tests/suites/anr/basic-session.js +++ b/packages/node-integration-tests/suites/anr/basic-session.js @@ -4,7 +4,6 @@ const Sentry = require('@sentry/node'); const { transport } = require('./test-transport.js'); -// close both processes after 5 seconds setTimeout(() => { process.exit(); }, 10000); diff --git a/packages/node-integration-tests/suites/anr/basic.js b/packages/node-integration-tests/suites/anr/basic.js index 47b2326410cb..563488beaf5a 100644 --- a/packages/node-integration-tests/suites/anr/basic.js +++ b/packages/node-integration-tests/suites/anr/basic.js @@ -4,7 +4,6 @@ const Sentry = require('@sentry/node'); const { transport } = require('./test-transport.js'); -// close both processes after 5 seconds setTimeout(() => { process.exit(); }, 10000); diff --git a/packages/node-integration-tests/suites/anr/basic.mjs b/packages/node-integration-tests/suites/anr/basic.mjs index cd858c4d1b4d..c03339411acb 100644 --- a/packages/node-integration-tests/suites/anr/basic.mjs +++ b/packages/node-integration-tests/suites/anr/basic.mjs @@ -4,7 +4,6 @@ import * as Sentry from '@sentry/node'; const { transport } = await import('./test-transport.js'); -// close both processes after 5 seconds setTimeout(() => { process.exit(); }, 10000); diff --git a/packages/node-integration-tests/suites/anr/forked.js b/packages/node-integration-tests/suites/anr/forked.js index 47b2326410cb..563488beaf5a 100644 --- a/packages/node-integration-tests/suites/anr/forked.js +++ b/packages/node-integration-tests/suites/anr/forked.js @@ -4,7 +4,6 @@ const Sentry = require('@sentry/node'); const { transport } = require('./test-transport.js'); -// close both processes after 5 seconds setTimeout(() => { process.exit(); }, 10000); From 3042553e706732a78901dc6030041f9f15da153d Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 12 Dec 2023 17:41:18 +0100 Subject: [PATCH 05/19] use getWorkerThreads --- packages/node/src/anr/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/node/src/anr/index.ts b/packages/node/src/anr/index.ts index 76d3e8296d5b..1d838ab6ad62 100644 --- a/packages/node/src/anr/index.ts +++ b/packages/node/src/anr/index.ts @@ -329,7 +329,7 @@ function handlerAnrWorker(options: Options): void { */ export function isAnrWorker(): boolean { try { - const { isMainThread } = dynamicRequire(module, 'worker_threads') as WorkerThreads; + const { isMainThread } = getWorkerThreads(); return !isMainThread; } catch (_) { return false; From 33e1ce0e1cc21e8e743e5835561a25a87c6bb884 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 13 Dec 2023 19:13:11 +0100 Subject: [PATCH 06/19] feat(node): Rework ANR to use worker script via an integration --- .gitignore | 3 + packages/core/src/index.ts | 2 +- .../suites/anr/basic-session.js | 24 +- .../suites/anr/basic.js | 24 +- .../suites/anr/basic.mjs | 6 +- .../suites/anr/forked.js | 24 +- .../suites/anr/test-transport.js | 17 - packages/node/rollup.npm.config.js | 31 +- packages/node/src/anr/index.ts | 391 ------------------ packages/node/src/index.ts | 1 - packages/node/src/integrations/anr/common.ts | 34 ++ packages/node/src/integrations/anr/index.ts | 172 ++++++++ packages/node/src/integrations/anr/worker.ts | 211 ++++++++++ packages/node/src/integrations/index.ts | 1 + packages/node/src/sdk.ts | 6 - packages/utils/src/anr.ts | 10 - rollup/bundleHelpers.js | 10 + 17 files changed, 492 insertions(+), 475 deletions(-) delete mode 100644 packages/node-integration-tests/suites/anr/test-transport.js delete mode 100644 packages/node/src/anr/index.ts create mode 100644 packages/node/src/integrations/anr/common.ts create mode 100644 packages/node/src/integrations/anr/index.ts create mode 100644 packages/node/src/integrations/anr/worker.ts diff --git a/.gitignore b/.gitignore index 813a38ad89d2..8d7413d26757 100644 --- a/.gitignore +++ b/.gitignore @@ -50,6 +50,9 @@ tmp.js .eslintcache **/eslintcache/* +# node worker scripts +packages/node/src/integrations/anr/worker-script.* + # deno packages/deno/build-types packages/deno/build-test diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index f261a2963364..f6f30661d48b 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -5,7 +5,7 @@ export type { ServerRuntimeClientOptions } from './server-runtime-client'; export type { RequestDataIntegrationOptions } from './integrations/requestdata'; export * from './tracing'; -export { createEventEnvelope } from './envelope'; +export { createEventEnvelope, createSessionEnvelope } from './envelope'; export { addBreadcrumb, captureCheckIn, diff --git a/packages/node-integration-tests/suites/anr/basic-session.js b/packages/node-integration-tests/suites/anr/basic-session.js index cfb2b7f05190..5d31281c67a4 100644 --- a/packages/node-integration-tests/suites/anr/basic-session.js +++ b/packages/node-integration-tests/suites/anr/basic-session.js @@ -2,8 +2,6 @@ const crypto = require('crypto'); const Sentry = require('@sentry/node'); -const { transport } = require('./test-transport.js'); - setTimeout(() => { process.exit(); }, 10000); @@ -12,19 +10,17 @@ Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', debug: true, - transport, + integrations: [new Sentry.Integrations.Anr({ captureStackTrace: true, anrThreshold: 200 })], }); -Sentry.enableAnrDetection({ captureStackTrace: true, anrThreshold: 200 }).then(() => { - function longWork() { - for (let i = 0; i < 100; i++) { - const salt = crypto.randomBytes(128).toString('base64'); - // eslint-disable-next-line no-unused-vars - const hash = crypto.pbkdf2Sync('myPassword', salt, 10000, 512, 'sha512'); - } +function longWork() { + for (let i = 0; i < 100; i++) { + const salt = crypto.randomBytes(128).toString('base64'); + // eslint-disable-next-line no-unused-vars + const hash = crypto.pbkdf2Sync('myPassword', salt, 10000, 512, 'sha512'); } +} - setTimeout(() => { - longWork(); - }, 1000); -}); +setTimeout(() => { + longWork(); +}, 1000); diff --git a/packages/node-integration-tests/suites/anr/basic.js b/packages/node-integration-tests/suites/anr/basic.js index 563488beaf5a..30d00a82d5f1 100644 --- a/packages/node-integration-tests/suites/anr/basic.js +++ b/packages/node-integration-tests/suites/anr/basic.js @@ -2,8 +2,6 @@ const crypto = require('crypto'); const Sentry = require('@sentry/node'); -const { transport } = require('./test-transport.js'); - setTimeout(() => { process.exit(); }, 10000); @@ -13,19 +11,17 @@ Sentry.init({ release: '1.0', debug: true, autoSessionTracking: false, - transport, + integrations: [new Sentry.Integrations.Anr({ captureStackTrace: true, anrThreshold: 200 })], }); -Sentry.enableAnrDetection({ captureStackTrace: true, anrThreshold: 200 }).then(() => { - function longWork() { - for (let i = 0; i < 100; i++) { - const salt = crypto.randomBytes(128).toString('base64'); - // eslint-disable-next-line no-unused-vars - const hash = crypto.pbkdf2Sync('myPassword', salt, 10000, 512, 'sha512'); - } +function longWork() { + for (let i = 0; i < 100; i++) { + const salt = crypto.randomBytes(128).toString('base64'); + // eslint-disable-next-line no-unused-vars + const hash = crypto.pbkdf2Sync('myPassword', salt, 10000, 512, 'sha512'); } +} - setTimeout(() => { - longWork(); - }, 1000); -}); +setTimeout(() => { + longWork(); +}, 1000); diff --git a/packages/node-integration-tests/suites/anr/basic.mjs b/packages/node-integration-tests/suites/anr/basic.mjs index c03339411acb..4e297c94b944 100644 --- a/packages/node-integration-tests/suites/anr/basic.mjs +++ b/packages/node-integration-tests/suites/anr/basic.mjs @@ -2,8 +2,6 @@ import * as crypto from 'crypto'; import * as Sentry from '@sentry/node'; -const { transport } = await import('./test-transport.js'); - setTimeout(() => { process.exit(); }, 10000); @@ -13,11 +11,9 @@ Sentry.init({ release: '1.0', debug: true, autoSessionTracking: false, - transport, + integrations: [new Sentry.Integrations.Anr({ captureStackTrace: true, anrThreshold: 200 })], }); -await Sentry.enableAnrDetection({ captureStackTrace: true, anrThreshold: 200 }); - 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/forked.js b/packages/node-integration-tests/suites/anr/forked.js index 563488beaf5a..30d00a82d5f1 100644 --- a/packages/node-integration-tests/suites/anr/forked.js +++ b/packages/node-integration-tests/suites/anr/forked.js @@ -2,8 +2,6 @@ const crypto = require('crypto'); const Sentry = require('@sentry/node'); -const { transport } = require('./test-transport.js'); - setTimeout(() => { process.exit(); }, 10000); @@ -13,19 +11,17 @@ Sentry.init({ release: '1.0', debug: true, autoSessionTracking: false, - transport, + integrations: [new Sentry.Integrations.Anr({ captureStackTrace: true, anrThreshold: 200 })], }); -Sentry.enableAnrDetection({ captureStackTrace: true, anrThreshold: 200 }).then(() => { - function longWork() { - for (let i = 0; i < 100; i++) { - const salt = crypto.randomBytes(128).toString('base64'); - // eslint-disable-next-line no-unused-vars - const hash = crypto.pbkdf2Sync('myPassword', salt, 10000, 512, 'sha512'); - } +function longWork() { + for (let i = 0; i < 100; i++) { + const salt = crypto.randomBytes(128).toString('base64'); + // eslint-disable-next-line no-unused-vars + const hash = crypto.pbkdf2Sync('myPassword', salt, 10000, 512, 'sha512'); } +} - setTimeout(() => { - longWork(); - }, 1000); -}); +setTimeout(() => { + longWork(); +}, 1000); diff --git a/packages/node-integration-tests/suites/anr/test-transport.js b/packages/node-integration-tests/suites/anr/test-transport.js deleted file mode 100644 index 86836cd6ab35..000000000000 --- a/packages/node-integration-tests/suites/anr/test-transport.js +++ /dev/null @@ -1,17 +0,0 @@ -const { TextEncoder, TextDecoder } = require('util'); - -const { createTransport } = require('@sentry/core'); -const { parseEnvelope } = require('@sentry/utils'); - -const textEncoder = new TextEncoder(); -const textDecoder = new TextDecoder(); - -// A transport that just logs the envelope payloads to console for checking in tests -exports.transport = () => { - return createTransport({ recordDroppedEvent: () => {}, textEncoder }, async request => { - const env = parseEnvelope(request.body, textEncoder, textDecoder); - // eslint-disable-next-line no-console - console.log(JSON.stringify(env[1][0][1])); - return { statusCode: 200 }; - }); -}; diff --git a/packages/node/rollup.npm.config.js b/packages/node/rollup.npm.config.js index 5a62b528ef44..8ee603a686ab 100644 --- a/packages/node/rollup.npm.config.js +++ b/packages/node/rollup.npm.config.js @@ -1,3 +1,30 @@ -import { makeBaseNPMConfig, makeNPMConfigVariants } from '../../rollup/index.js'; +import { makeBaseBundleConfig, makeBaseNPMConfig, makeNPMConfigVariants } from '../../rollup/index.js'; -export default makeNPMConfigVariants(makeBaseNPMConfig()); +const anrWorkerConfig = makeBaseBundleConfig({ + bundleType: 'node-worker', + entrypoints: ['src/integrations/anr/worker.ts'], + jsVersion: 'es6', + licenseTitle: '@sentry/node', + outputFileBase: () => 'worker-script.js.ignore', + packageSpecificConfig: { + output: { + dir: './src/integrations/anr', + sourcemap: false, + }, + plugins: [ + { + name: 'output-worker-script', + generateBundle(_, bundle) { + const entry = Object.values(bundle).find(chunk => chunk.isEntry); + this.emitFile({ + type: 'asset', + fileName: 'worker-script.ts', + source: `export const base64WorkerScript = '${Buffer.from(entry.code).toString('base64')}';`, + }); + }, + }, + ], + }, +}); + +export default [anrWorkerConfig, ...makeNPMConfigVariants(makeBaseNPMConfig())]; diff --git a/packages/node/src/anr/index.ts b/packages/node/src/anr/index.ts deleted file mode 100644 index 1d838ab6ad62..000000000000 --- a/packages/node/src/anr/index.ts +++ /dev/null @@ -1,391 +0,0 @@ -import { getClient, makeSession, updateSession } from '@sentry/core'; -import type { Event, Session, StackFrame, TraceContext } from '@sentry/types'; -import { - callFrameToStackFrame, - dynamicRequire, - logger, - stripSentryFramesAndReverse, - watchdogTimer, -} from '@sentry/utils'; -import type { Session as InspectorSession } from 'inspector'; - -import type { MessagePort, Worker } from 'worker_threads'; -import { addEventProcessor, captureEvent, flush, getCurrentHub, getModuleFromFilename } from '..'; -import { NODE_VERSION } from '../nodeVersion'; - -const DEFAULT_INTERVAL = 50; -const DEFAULT_HANG_THRESHOLD = 5000; - -interface Options { - /** - * The app entry script. This is used to run the same script as the ANR worker. - * - * Defaults to `process.argv[1]`. - */ - entryScript: string; - /** - * Interval to send heartbeat messages to the ANR worker. - * - * Defaults to 50ms. - */ - pollInterval: number; - /** - * Threshold in milliseconds to trigger an ANR event. - * - * Defaults to 5000ms. - */ - anrThreshold: number; - /** - * Whether to capture a stack trace when the ANR event is triggered. - * - * Defaults to `false`. - * - * This uses the node debugger which enables the inspector API and opens the required ports. - */ - captureStackTrace: boolean; - /** - * @deprecated Use 'init' debug option instead - */ - debug: boolean; -} - -function createAnrEvent(blockedMs: number, frames?: StackFrame[]): Event { - return { - level: 'error', - exception: { - values: [ - { - type: 'ApplicationNotResponding', - value: `Application Not Responding for at least ${blockedMs} ms`, - stacktrace: { frames }, - mechanism: { - // This ensures the UI doesn't say 'Crashed in' for the stack trace - type: 'ANR', - }, - }, - ], - }, - }; -} - -type WorkerThreads = { - Worker: typeof Worker; - isMainThread: boolean; - parentPort: null | MessagePort; - workerData: { inspectorUrl?: string }; -}; - -/** - * We need to use dynamicRequire because worker_threads is not available in node < v12 and webpack error will when - * targeting those versions - */ -function getWorkerThreads(): WorkerThreads { - return dynamicRequire(module, 'worker_threads'); -} - -type InspectorSessionNodeV12 = InspectorSession & { connectToMainThread: () => void }; - -interface InspectorApi { - open: (port: number) => void; - url: () => string | undefined; -} - -/** - * Starts the node debugger and returns the inspector url. - * - * When inspector.url() returns undefined, it means the port is already in use so we try the next port. - */ -function startInspector(startPort: number = 9229): string | undefined { - // eslint-disable-next-line @typescript-eslint/no-var-requires - const inspector: InspectorApi = require('inspector'); - let inspectorUrl: string | undefined = undefined; - let port = startPort; - - while (inspectorUrl === undefined && port < startPort + 100) { - inspector.open(port); - inspectorUrl = inspector.url(); - port++; - } - - return inspectorUrl; -} - -function startAnrWorker(options: Options): void { - const { Worker } = getWorkerThreads(); - - function log(message: string, ...args: unknown[]): void { - logger.log(`[ANR] ${message}`, ...args); - } - - const hub = getCurrentHub(); - - try { - log(`Spawning worker with entryScript:'${options.entryScript}'`); - - const inspectorUrl = options.captureStackTrace ? startInspector() : undefined; - const worker = new Worker(options.entryScript, { workerData: { inspectorUrl } }); - // The worker should not keep the main process alive - worker.unref(); - - const timer = setInterval(() => { - try { - const currentSession = hub.getScope()?.getSession(); - // We need to copy the session object and remove the toJSON method so it can be sent to the worker - // serialized without making it a SerializedSession - const session = currentSession ? { ...currentSession, toJSON: undefined } : undefined; - // message the worker to tell it the main event loop is still running - worker.postMessage({ session }); - } catch (_) { - // - } - }, options.pollInterval); - - worker.on('message', (msg: string) => { - if (msg === 'session-ended') { - log('ANR event sent from ANR worker. Clearing session in this thread.'); - hub.getScope()?.setSession(undefined); - } - }); - - worker.once('error', (err: Error) => { - clearInterval(timer); - log('ANR worker error', err); - }); - - worker.once('exit', (code: number) => { - clearInterval(timer); - log('ANR worker exit', code); - }); - } catch (e) { - log('Failed to start worker', e); - } -} - -function createHrTimer(): { getTimeMs: () => number; reset: () => void } { - let lastPoll = process.hrtime(); - - return { - getTimeMs: (): number => { - const [seconds, nanoSeconds] = process.hrtime(lastPoll); - return Math.floor(seconds * 1e3 + nanoSeconds / 1e6); - }, - reset: (): void => { - lastPoll = process.hrtime(); - }, - }; -} - -function handlerAnrWorker(options: Options): void { - const { parentPort, workerData } = getWorkerThreads(); - - let anrEventSent = false; - - function log(message: string): void { - logger.log(`[ANR Worker] ${message}`); - } - - log('Started'); - let session: Session | undefined; - - function sendAnrEvent(frames?: StackFrame[], traceContext?: TraceContext): void { - if (anrEventSent) { - return; - } - - anrEventSent = true; - - log('Sending ANR event'); - - if (session) { - log('Sending abnormal session'); - updateSession(session, { status: 'abnormal', abnormal_mechanism: 'anr_foreground' }); - getClient()?.sendSession(session); - - try { - // Notify the main process that the session has ended so the session can be cleared from the scope - parentPort?.postMessage('session-ended'); - } catch (_) { - // ignore - } - } - - captureEvent(createAnrEvent(options.anrThreshold, frames), { - captureContext: { contexts: { trace: traceContext } }, - }); - - void flush(3000).then(() => { - // We exit so we only capture one event to avoid spamming users with errors - // We wait 5 seconds to ensure stdio has been flushed from the worker - setTimeout(() => { - process.exit(); - }, 5_000); - }); - } - - addEventProcessor(event => { - // Strip sdkProcessingMetadata from all ANR worker events to remove trace info - delete event.sdkProcessingMetadata; - event.tags = { - ...event.tags, - 'process.name': 'ANR', - }; - return event; - }); - - let debuggerPause: () => void | undefined; - - const { inspectorUrl } = workerData; - - // if attachStackTrace was enabled, we'll have a debugger url to connect to - if (inspectorUrl) { - log('Connecting to debugger'); - - // eslint-disable-next-line @typescript-eslint/no-var-requires - const { Session } = require('inspector'); - const session: InspectorSessionNodeV12 = new Session(); - session.connectToMainThread(); - - // Collect scriptId -> url map so we can look up the filenames later - const scripts = new Map(); - - session.on('Debugger.scriptParsed', event => { - scripts.set(event.params.scriptId, event.params.url); - }); - - session.on('Debugger.paused', event => { - if (event.params.reason !== 'other') { - return; - } - - // copy the frames - const callFrames = [...event.params.callFrames]; - - const stackFrames = stripSentryFramesAndReverse( - callFrames.map(frame => - callFrameToStackFrame(frame, scripts.get(frame.location.scriptId), getModuleFromFilename), - ), - ); - - // Evaluate a script in the currently paused context - session.post( - 'Runtime.evaluate', - { - // Grab the trace context from the current scope - expression: 'const ctx = __SENTRY__.hub.getScope().getPropagationContext(); ctx.traceId + "-" + ctx.spanId', - // Don't re-trigger the debugger if this causes an error - silent: true, - }, - (_, param) => { - const traceId = param && param.result ? (param.result.value as string) : '-'; - const [trace_id, span_id] = traceId.split('-'); - - // Resume immediately - session.post('Debugger.resume'); - session.post('Debugger.disable'); - - const context = trace_id && span_id ? { trace_id, span_id } : undefined; - sendAnrEvent(stackFrames, context); - }, - ); - }); - - debuggerPause = () => { - session.post('Debugger.enable', () => { - session.post('Debugger.pause'); - }); - }; - } - - async function watchdogTimeout(): Promise { - log('Watchdog timeout'); - - try { - if (debuggerPause) { - log('Pausing debugger to capture stack trace'); - debuggerPause(); - return; - } - } catch (_) { - // ignore - } - - log('Capturing event without a stack trace'); - sendAnrEvent(); - } - - const { poll } = watchdogTimer(createHrTimer, options.pollInterval, options.anrThreshold, watchdogTimeout); - - parentPort?.on('message', (msg: { session: Session | undefined }) => { - if (msg.session) { - session = makeSession(msg.session); - } - - poll(); - }); -} - -/** - * Returns true if the current thread is the ANR worker. - */ -export function isAnrWorker(): boolean { - try { - const { isMainThread } = getWorkerThreads(); - return !isMainThread; - } catch (_) { - return false; - } -} - -/** - * **Note** This feature is still in beta so there may be breaking changes in future releases. - * - * Starts a ANR worker that detects Application Not Responding (ANR) errors. - * - * It's important to await on the returned promise before your app code to ensure this code does not run in the - * ANR worker. - * - * ```js - * import { init, enableAnrDetection } from '@sentry/node'; - * - * init({ dsn: "__DSN__" }); - * - * // with ESM + Node 14+ - * await enableAnrDetection({ captureStackTrace: true }); - * runApp(); - * - * // with CJS or Node 10+ - * enableAnrDetection({ captureStackTrace: true }).then(() => { - * runApp(); - * }); - * ``` - */ -export function enableAnrDetection(options: Partial): Promise { - if ((NODE_VERSION.major || 0) < 12 || ((NODE_VERSION.major || 0) === 12 && (NODE_VERSION.minor || 0) < 11)) { - throw new Error('ANR detection requires Node 12.11.0 or later'); - } - - // When pm2 runs the script in cluster mode, process.argv[1] is the pm2 script and process.env.pm_exec_path is the - // path to the entry script - const entryScript = options.entryScript || process.env.pm_exec_path || process.argv[1]; - - const anrOptions: Options = { - entryScript, - pollInterval: options.pollInterval || DEFAULT_INTERVAL, - anrThreshold: options.anrThreshold || DEFAULT_HANG_THRESHOLD, - captureStackTrace: !!options.captureStackTrace, - // eslint-disable-next-line deprecation/deprecation - debug: !!options.debug, - }; - - if (isAnrWorker()) { - handlerAnrWorker(anrOptions); - // In the ANR worker, the promise never resolves which stops the app code from running - return new Promise(() => { - // Never resolve - }); - } else { - startAnrWorker(anrOptions); - // In the main process, the promise resolves immediately - return Promise.resolve(); - } -} diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index 9963258e48bb..3324916ad303 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -78,7 +78,6 @@ export { defaultIntegrations, init, defaultStackParser, getSentryRelease } from export { addRequestDataToEvent, DEFAULT_USER_INCLUDES, extractRequestData } from '@sentry/utils'; export { deepReadDirSync } from './utils'; export { getModuleFromFilename } from './module'; -export { enableAnrDetection } from './anr'; import { Integrations as CoreIntegrations } from '@sentry/core'; diff --git a/packages/node/src/integrations/anr/common.ts b/packages/node/src/integrations/anr/common.ts new file mode 100644 index 000000000000..38583dfacaaf --- /dev/null +++ b/packages/node/src/integrations/anr/common.ts @@ -0,0 +1,34 @@ +import type { Contexts, DsnComponents, SdkMetadata } from '@sentry/types'; + +export interface Options { + /** + * Interval to send heartbeat messages to the ANR worker. + * + * Defaults to 50ms. + */ + pollInterval: number; + /** + * Threshold in milliseconds to trigger an ANR event. + * + * Defaults to 5000ms. + */ + anrThreshold: number; + /** + * Whether to capture a stack trace when the ANR event is triggered. + * + * Defaults to `false`. + * + * This uses the node debugger which enables the inspector API and opens the required ports. + */ + captureStackTrace: boolean; +} + +export interface WorkerStartData extends Options { + debug: boolean; + sdkMetadata: SdkMetadata; + dsn: DsnComponents; + release: string | undefined; + environment: string; + dist: string | undefined; + contexts: Contexts; +} diff --git a/packages/node/src/integrations/anr/index.ts b/packages/node/src/integrations/anr/index.ts new file mode 100644 index 000000000000..c6919b22e6d8 --- /dev/null +++ b/packages/node/src/integrations/anr/index.ts @@ -0,0 +1,172 @@ +import { URL } from 'url'; +import { getCurrentHub } from '@sentry/core'; +import type { Contexts, Event, EventHint, Integration } from '@sentry/types'; +import { dynamicRequire, logger } from '@sentry/utils'; +import type { Worker, WorkerOptions } from 'worker_threads'; +import type { NodeClient } from '../../client'; +import { NODE_VERSION } from '../../nodeVersion'; +import type { Options, WorkerStartData } from './common'; +import { base64WorkerScript } from './worker-script'; + +const DEFAULT_INTERVAL = 50; +const DEFAULT_HANG_THRESHOLD = 5000; + +type WorkerNodeV14 = Worker & { new (filename: string | URL, options?: WorkerOptions): Worker }; + +type WorkerThreads = { + Worker: WorkerNodeV14; +}; + +function log(message: string, ...args: unknown[]): void { + logger.log(`[ANR] ${message}`, ...args); +} + +/** + * We need to use dynamicRequire because worker_threads is not available in node < v12 and webpack error will when + * targeting those versions + */ +function getWorkerThreads(): WorkerThreads { + return dynamicRequire(module, 'worker_threads'); +} + +/** + * Gets contexts by calling all event processors. This relies on being called after all integrations are setup + */ +async function getContexts(client: NodeClient): Promise { + let event: Event | null = { message: 'ANR' }; + const eventHint: EventHint = {}; + + for (const processor of client.getEventProcessors()) { + if (event === null) break; + event = await processor(event, eventHint); + } + + return event?.contexts || {}; +} + +interface InspectorApi { + open: (port: number) => void; + url: () => string | undefined; +} + +/** + * Starts the node debugger and returns the inspector url. + * + * When inspector.url() returns undefined, it means the port is already in use so we try the next port. + */ +function startInspector(startPort: number = 9229): string | undefined { + // eslint-disable-next-line @typescript-eslint/no-var-requires + const inspector: InspectorApi = require('inspector'); + let inspectorUrl: string | undefined = undefined; + let port = startPort; + + while (inspectorUrl === undefined && port < startPort + 100) { + inspector.open(port); + inspectorUrl = inspector.url(); + port++; + } + + return inspectorUrl; +} + +/** + * Starts a thread to detect App Not Responding (ANR) events + */ +export class Anr implements Integration { + public name: string = 'Anr'; + + public constructor(private readonly _options: Partial = {}) {} + + /** @inheritdoc */ + public setupOnce(): void { + // + } + + /** @inheritdoc */ + public setup(client: NodeClient): void { + if ((NODE_VERSION.major || 0) < 14 || ((NODE_VERSION.major || 0) === 14 && (NODE_VERSION.minor || 0) < 7)) { + throw new Error('ANR detection requires Node 14.7.0 or later'); + } + + // setImmediate is used to ensure that all other integrations have been setup + setImmediate(() => this._startWorker(client)); + } + + /** + * Starts the ANR worker thread + */ + private async _startWorker(client: NodeClient): Promise { + const contexts = await getContexts(client); + const initOptions = client.getOptions(); + + const dsn = client.getDsn(); + + if (!dsn) { + return; + } + + // These will not be inaccurate if sent later from the worker thread + delete contexts.app?.app_memory; + delete contexts.device?.free_memory; + + const sdkMetadata = client.getSdkMetadata() || {}; + if (sdkMetadata.sdk) { + sdkMetadata.sdk.integrations = client.getOptions().integrations.map(i => i.name); + } + + const options: WorkerStartData = { + debug: logger.isEnabled(), + dsn, + environment: initOptions.environment || 'production', + release: initOptions.release, + dist: initOptions.dist, + sdkMetadata, + pollInterval: this._options.pollInterval || DEFAULT_INTERVAL, + anrThreshold: this._options.anrThreshold || DEFAULT_HANG_THRESHOLD, + captureStackTrace: !!this._options.captureStackTrace, + contexts, + }; + + if (options.captureStackTrace) { + startInspector(); + } + + const { Worker } = getWorkerThreads(); + + const worker = new Worker(new URL(`data:application/javascript;base64,${base64WorkerScript}`), { + workerData: options, + }); + // Ensure this thread can't block app exit + worker.unref(); + + const timer = setInterval(() => { + try { + const currentSession = getCurrentHub().getScope()?.getSession(); + // We need to copy the session object and remove the toJSON method so it can be sent to the worker + // serialized without making it a SerializedSession + const session = currentSession ? { ...currentSession, toJSON: undefined } : undefined; + // message the worker to tell it the main event loop is still running + worker.postMessage({ session }); + } catch (_) { + // + } + }, options.pollInterval); + + worker.on('message', (msg: string) => { + if (msg === 'session-ended') { + log('ANR event sent from ANR worker. Clearing session in this thread.'); + getCurrentHub().getScope()?.setSession(undefined); + } + }); + + worker.once('error', (err: Error) => { + clearInterval(timer); + log('ANR worker error', err); + }); + + worker.once('exit', (code: number) => { + clearInterval(timer); + log('ANR worker exit', code); + }); + } +} diff --git a/packages/node/src/integrations/anr/worker.ts b/packages/node/src/integrations/anr/worker.ts new file mode 100644 index 000000000000..5b77c44d8bc6 --- /dev/null +++ b/packages/node/src/integrations/anr/worker.ts @@ -0,0 +1,211 @@ +import { + createEventEnvelope, + createSessionEnvelope, + getEnvelopeEndpointWithUrlEncodedAuth, + makeSession, + updateSession, +} from '@sentry/core'; +import type { Event, Session, StackFrame, TraceContext, Transport } from '@sentry/types'; +import { callFrameToStackFrame, stripSentryFramesAndReverse, watchdogTimer } from '@sentry/utils'; +import { Session as InspectorSession } from 'inspector'; +import { parentPort, workerData } from 'worker_threads'; +import { makeNodeTransport } from '../../transports'; +import type { WorkerStartData } from './common'; + +type InspectorSessionNodeV12 = InspectorSession & { connectToMainThread: () => void }; + +const options: WorkerStartData = workerData; +let session: Session | undefined; +let hasSentAnrEvent = false; + +function log(msg: string): void { + if (options.debug) { + // eslint-disable-next-line no-console + console.log(`[ANR Worker] ${msg}`); + } +} + +function getTransport(): Transport { + const url = getEnvelopeEndpointWithUrlEncodedAuth(options.dsn); + return makeNodeTransport({ + url, + recordDroppedEvent: () => { + // + }, + }); +} + +async function sendAbnormalSession(): Promise { + // of we have an existing session passed from the main thread, send it as abnormal + if (session) { + log('Sending abnormal session'); + updateSession(session, { status: 'abnormal', abnormal_mechanism: 'anr_foreground' }); + const envelope = createSessionEnvelope(session, options.dsn, options.sdkMetadata); + await getTransport().send(envelope); + + try { + // Notify the main process that the session has ended so the session can be cleared from the scope + parentPort?.postMessage('session-ended'); + } catch (_) { + // ignore + } + } +} + +log('Started'); + +async function sendAnrEvent(frames?: StackFrame[], traceContext?: TraceContext): Promise { + if (hasSentAnrEvent) { + return; + } + + hasSentAnrEvent = true; + + await sendAbnormalSession(); + + log('Sending event'); + + const event: Event = { + sdk: options.sdkMetadata.sdk, + contexts: { ...options.contexts, trace: traceContext }, + release: options.release, + environment: options.environment, + dist: options.dist, + level: 'error', + exception: { + values: [ + { + type: 'ApplicationNotResponding', + value: `Application Not Responding for at least ${options.anrThreshold} ms`, + stacktrace: { frames }, + // This ensures the UI doesn't say 'Crashed in' for the stack trace + mechanism: { type: 'ANR' }, + }, + ], + }, + tags: { 'process.name': 'ANR' }, + }; + + if (options.debug) { + // eslint-disable-next-line no-console + console.log(JSON.stringify(event)); + } + + const envelope = createEventEnvelope(event, options.dsn, options.sdkMetadata); + const transport = getTransport(); + + await transport.send(envelope); + await transport.flush(2000); + + // Delay for 5 seconds so that stdio can flush in the main event loop ever restarts. + // This is mainly for the benefit of logging/debugging issues. + setTimeout(() => { + process.exit(0); + }, 5_000); +} + +let debuggerPause: () => void | undefined; + +if (options.captureStackTrace) { + log('Connecting to debugger'); + + const session = new InspectorSession() as InspectorSessionNodeV12; + session.connectToMainThread(); + + log('Connected to debugger'); + + // Collect scriptId -> url map so we can look up the filenames later + const scripts = new Map(); + + session.on('Debugger.scriptParsed', event => { + scripts.set(event.params.scriptId, event.params.url); + }); + + session.on('Debugger.paused', event => { + if (event.params.reason !== 'other') { + return; + } + + try { + log('Debugger paused'); + + // copy the frames + const callFrames = [...event.params.callFrames]; + + const stackFrames = stripSentryFramesAndReverse( + callFrames.map(frame => callFrameToStackFrame(frame, scripts.get(frame.location.scriptId), () => undefined)), + ); + + // Evaluate a script in the currently paused context + session.post( + 'Runtime.evaluate', + { + // Grab the trace context from the current scope + expression: 'const ctx = __SENTRY__.hub.getScope().getPropagationContext(); ctx.traceId + "-" + ctx.spanId', + // Don't re-trigger the debugger if this causes an error + silent: true, + }, + (_, param) => { + const traceId = param && param.result ? (param.result.value as string) : '-'; + const [trace_id, span_id] = traceId.split('-'); + + session.post('Debugger.resume'); + session.post('Debugger.disable'); + + const context = trace_id && span_id ? { trace_id, span_id } : undefined; + void sendAnrEvent(stackFrames, context); + }, + ); + } catch (e) { + session.post('Debugger.resume'); + session.post('Debugger.disable'); + throw e; + } + }); + + debuggerPause = () => { + try { + session.post('Debugger.enable', () => { + session.post('Debugger.pause'); + }); + } catch (_) { + // + } + }; +} + +function createHrTimer(): { getTimeMs: () => number; reset: () => void } { + let lastPoll = process.hrtime(); + + return { + getTimeMs: (): number => { + const [seconds, nanoSeconds] = process.hrtime(lastPoll); + return Math.floor(seconds * 1e3 + nanoSeconds / 1e6); + }, + reset: (): void => { + lastPoll = process.hrtime(); + }, + }; +} + +function watchdogTimeout(): void { + log('Watchdog timeout'); + + if (debuggerPause) { + log('Pausing debugger to capture stack trace'); + debuggerPause(); + } else { + log('Capturing event without a stack trace'); + void sendAnrEvent(); + } +} + +const { poll } = watchdogTimer(createHrTimer, options.pollInterval, options.anrThreshold, watchdogTimeout); + +parentPort?.on('message', (msg: { session: Session | undefined }) => { + if (msg.session) { + session = makeSession(msg.session); + } + + poll(); +}); diff --git a/packages/node/src/integrations/index.ts b/packages/node/src/integrations/index.ts index 49820882fdc6..25d3572a7b16 100644 --- a/packages/node/src/integrations/index.ts +++ b/packages/node/src/integrations/index.ts @@ -9,3 +9,4 @@ export { RequestData } from '@sentry/core'; export { LocalVariables } from './localvariables'; export { Undici } from './undici'; export { Spotlight } from './spotlight'; +export { Anr } from './anr'; diff --git a/packages/node/src/sdk.ts b/packages/node/src/sdk.ts index d8498ef7ecde..790031451b3f 100644 --- a/packages/node/src/sdk.ts +++ b/packages/node/src/sdk.ts @@ -15,7 +15,6 @@ import { tracingContextFromHeaders, } from '@sentry/utils'; -import { isAnrWorker } from './anr'; import { setNodeAsyncContextStrategy } from './async'; import { NodeClient } from './client'; import { @@ -112,11 +111,6 @@ export const defaultIntegrations = [ */ // eslint-disable-next-line complexity export function init(options: NodeOptions = {}): void { - if (isAnrWorker()) { - options.autoSessionTracking = false; - options.tracesSampleRate = 0; - } - const carrier = getMainCarrier(); setNodeAsyncContextStrategy(); diff --git a/packages/utils/src/anr.ts b/packages/utils/src/anr.ts index 5312d5632ac9..d962107bcb0e 100644 --- a/packages/utils/src/anr.ts +++ b/packages/utils/src/anr.ts @@ -67,16 +67,6 @@ interface CallFrame { url: string; } -interface ScriptParsedEventDataType { - scriptId: string; - url: string; -} - -interface PausedEventDataType { - callFrames: CallFrame[]; - reason: string; -} - /** * Converts Debugger.CallFrame to Sentry StackFrame */ diff --git a/rollup/bundleHelpers.js b/rollup/bundleHelpers.js index cd329dfb31d2..104c0c61e83b 100644 --- a/rollup/bundleHelpers.js +++ b/rollup/bundleHelpers.js @@ -102,6 +102,15 @@ export function makeBaseBundleConfig(options) { external: builtinModules, }; + const workerBundleConfig = { + output: { + format: 'esm', + }, + plugins: [commonJSPlugin, makeTerserPlugin()], + // Don't bundle any of Node's core modules + external: builtinModules, + }; + // used by all bundles const sharedBundleConfig = { input: entrypoints, @@ -123,6 +132,7 @@ export function makeBaseBundleConfig(options) { standalone: standAloneBundleConfig, addon: addOnBundleConfig, node: nodeBundleConfig, + 'node-worker': workerBundleConfig, }; return deepMerge.all([sharedBundleConfig, bundleTypeConfigMap[bundleType], packageSpecificConfig || {}], { From 97b09bb7f60a450d5c6676bc11e2c35e9db665d0 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 13 Dec 2023 19:31:08 +0100 Subject: [PATCH 07/19] Bodge the build and add platform --- packages/node/package.json | 2 +- packages/node/src/integrations/anr/worker.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/node/package.json b/packages/node/package.json index de1b58fb3f86..047b1a133269 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -42,7 +42,7 @@ "build": "run-p build:transpile build:types", "build:dev": "yarn build", "build:transpile": "rollup -c rollup.npm.config.js", - "build:types": "run-s build:types:core build:types:downlevel", + "build:types": "run-s build:transpile build:types:core build:types:downlevel", "build:types:core": "tsc -p tsconfig.types.json", "build:types:downlevel": "yarn downlevel-dts build/types build/types-ts3.8 --to ts3.8", "build:watch": "run-p build:transpile:watch build:types:watch", diff --git a/packages/node/src/integrations/anr/worker.ts b/packages/node/src/integrations/anr/worker.ts index 5b77c44d8bc6..6542ead3b18a 100644 --- a/packages/node/src/integrations/anr/worker.ts +++ b/packages/node/src/integrations/anr/worker.ts @@ -71,6 +71,7 @@ async function sendAnrEvent(frames?: StackFrame[], traceContext?: TraceContext): release: options.release, environment: options.environment, dist: options.dist, + platform: 'node', level: 'error', exception: { values: [ From b2a190731dc39c3f2d803e102c04bb9f01bb0145 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 13 Dec 2023 19:50:40 +0100 Subject: [PATCH 08/19] also build before test --- packages/node/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/node/package.json b/packages/node/package.json index 047b1a133269..3b7b9a616e20 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -54,7 +54,7 @@ "clean": "rimraf build coverage sentry-node-*.tgz", "fix": "eslint . --format stylish --fix", "lint": "eslint . --format stylish", - "test": "run-s test:jest test:express test:webpack test:release-health", + "test": "run-s build:transpile test:jest test:express test:webpack test:release-health", "test:express": "node test/manual/express-scope-separation/start.js", "test:jest": "jest", "test:release-health": "node test/manual/release-health/runner.js", From 8b6289f96f53c7d5d590d09df622f9dfc3e774b9 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 13 Dec 2023 20:25:46 +0100 Subject: [PATCH 09/19] Work on build --- .../node-integration-tests/suites/anr/test.ts | 6 ++-- packages/node/.eslintrc.js | 1 + packages/node/package.json | 8 +++-- packages/node/rollup.anr-worker.config.js | 23 ++++++++++++++ packages/node/rollup.npm.config.js | 31 ++----------------- packages/node/src/integrations/anr/worker.ts | 6 ++++ 6 files changed, 40 insertions(+), 35 deletions(-) create mode 100644 packages/node/rollup.anr-worker.config.js diff --git a/packages/node-integration-tests/suites/anr/test.ts b/packages/node-integration-tests/suites/anr/test.ts index de4032479f03..8f0ca9dc98ce 100644 --- a/packages/node-integration-tests/suites/anr/test.ts +++ b/packages/node-integration-tests/suites/anr/test.ts @@ -26,7 +26,7 @@ function parseJsonLines(input: string, expected: number): T describe('should report ANR when event loop blocked', () => { test('CJS', done => { - if (NODE_VERSION < 12) { + if (NODE_VERSION < 14) { done(); return; } @@ -78,7 +78,7 @@ describe('should report ANR when event loop blocked', () => { }); test('With session', done => { - if (NODE_VERSION < 12) { + if (NODE_VERSION < 14) { done(); return; } @@ -106,7 +106,7 @@ describe('should report ANR when event loop blocked', () => { }); test('from forked process', done => { - if (NODE_VERSION < 12) { + if (NODE_VERSION < 14) { done(); return; } diff --git a/packages/node/.eslintrc.js b/packages/node/.eslintrc.js index bec6469d0e28..6b31883cc1bc 100644 --- a/packages/node/.eslintrc.js +++ b/packages/node/.eslintrc.js @@ -2,6 +2,7 @@ module.exports = { env: { node: true, }, + ignorePatterns: ['src/integrations/anr/worker-script.ts'], extends: ['../../.eslintrc.js'], rules: { '@sentry-internal/sdk/no-optional-chaining': 'off', diff --git a/packages/node/package.json b/packages/node/package.json index 3b7b9a616e20..46ca406f668b 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -41,8 +41,8 @@ "scripts": { "build": "run-p build:transpile build:types", "build:dev": "yarn build", - "build:transpile": "rollup -c rollup.npm.config.js", - "build:types": "run-s build:transpile build:types:core build:types:downlevel", + "build:transpile": "yarn build:anr-worker && rollup -c rollup.npm.config.js", + "build:types": "run-s build:anr-worker build:types:core build:types:downlevel", "build:types:core": "tsc -p tsconfig.types.json", "build:types:downlevel": "yarn downlevel-dts build/types build/types-ts3.8 --to ts3.8", "build:watch": "run-p build:transpile:watch build:types:watch", @@ -50,11 +50,13 @@ "build:transpile:watch": "rollup -c rollup.npm.config.js --watch", "build:types:watch": "tsc -p tsconfig.types.json --watch", "build:tarball": "ts-node ../../scripts/prepack.ts && npm pack ./build", + "build:anr-worker": "rollup -c rollup.anr-worker.config.js", + "build:anr-worker-stub": "echo 'export const base64WorkerScript=\"\";' > src/integrations/anr/worker-script.ts", "circularDepCheck": "madge --circular src/index.ts", "clean": "rimraf build coverage sentry-node-*.tgz", "fix": "eslint . --format stylish --fix", "lint": "eslint . --format stylish", - "test": "run-s build:transpile test:jest test:express test:webpack test:release-health", + "test": "run-s build:anr-worker-stub test:jest test:express test:webpack test:release-health", "test:express": "node test/manual/express-scope-separation/start.js", "test:jest": "jest", "test:release-health": "node test/manual/release-health/runner.js", diff --git a/packages/node/rollup.anr-worker.config.js b/packages/node/rollup.anr-worker.config.js new file mode 100644 index 000000000000..acac8ea1a34f --- /dev/null +++ b/packages/node/rollup.anr-worker.config.js @@ -0,0 +1,23 @@ +import { makeBaseBundleConfig } from '../../rollup/index.js'; + +export default makeBaseBundleConfig({ + bundleType: 'node-worker', + entrypoints: ['src/integrations/anr/worker.ts'], + jsVersion: 'es6', + licenseTitle: '@sentry/node', + outputFileBase: () => 'worker-script.ts', + packageSpecificConfig: { + output: { + dir: './src/integrations/anr', + sourcemap: false, + }, + plugins: [ + { + name: 'output-base64-worker-script', + renderChunk(code) { + return `export const base64WorkerScript = '${Buffer.from(code).toString('base64')}';`; + }, + }, + ], + }, +}); diff --git a/packages/node/rollup.npm.config.js b/packages/node/rollup.npm.config.js index 8ee603a686ab..5a62b528ef44 100644 --- a/packages/node/rollup.npm.config.js +++ b/packages/node/rollup.npm.config.js @@ -1,30 +1,3 @@ -import { makeBaseBundleConfig, makeBaseNPMConfig, makeNPMConfigVariants } from '../../rollup/index.js'; +import { makeBaseNPMConfig, makeNPMConfigVariants } from '../../rollup/index.js'; -const anrWorkerConfig = makeBaseBundleConfig({ - bundleType: 'node-worker', - entrypoints: ['src/integrations/anr/worker.ts'], - jsVersion: 'es6', - licenseTitle: '@sentry/node', - outputFileBase: () => 'worker-script.js.ignore', - packageSpecificConfig: { - output: { - dir: './src/integrations/anr', - sourcemap: false, - }, - plugins: [ - { - name: 'output-worker-script', - generateBundle(_, bundle) { - const entry = Object.values(bundle).find(chunk => chunk.isEntry); - this.emitFile({ - type: 'asset', - fileName: 'worker-script.ts', - source: `export const base64WorkerScript = '${Buffer.from(entry.code).toString('base64')}';`, - }); - }, - }, - ], - }, -}); - -export default [anrWorkerConfig, ...makeNPMConfigVariants(makeBaseNPMConfig())]; +export default makeNPMConfigVariants(makeBaseNPMConfig()); diff --git a/packages/node/src/integrations/anr/worker.ts b/packages/node/src/integrations/anr/worker.ts index 6542ead3b18a..7b466d1cae08 100644 --- a/packages/node/src/integrations/anr/worker.ts +++ b/packages/node/src/integrations/anr/worker.ts @@ -40,6 +40,12 @@ async function sendAbnormalSession(): Promise { if (session) { log('Sending abnormal session'); updateSession(session, { status: 'abnormal', abnormal_mechanism: 'anr_foreground' }); + + if (options.debug) { + // eslint-disable-next-line no-console + console.log(JSON.stringify(session)); + } + const envelope = createSessionEnvelope(session, options.dsn, options.sdkMetadata); await getTransport().send(envelope); From 91c19b94b3a31e94e8d2f00bf1c89cf749628394 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 13 Dec 2023 20:41:14 +0100 Subject: [PATCH 10/19] =?UTF-8?q?Only=20node=2016=20=F0=9F=98=A2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/node-integration-tests/suites/anr/test.ts | 8 ++++---- packages/node/src/integrations/anr/index.ts | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/node-integration-tests/suites/anr/test.ts b/packages/node-integration-tests/suites/anr/test.ts index 8f0ca9dc98ce..a6ef0c4046b4 100644 --- a/packages/node-integration-tests/suites/anr/test.ts +++ b/packages/node-integration-tests/suites/anr/test.ts @@ -26,7 +26,7 @@ function parseJsonLines(input: string, expected: number): T describe('should report ANR when event loop blocked', () => { test('CJS', done => { - if (NODE_VERSION < 14) { + if (NODE_VERSION < 16) { done(); return; } @@ -54,7 +54,7 @@ describe('should report ANR when event loop blocked', () => { }); test('ESM', done => { - if (NODE_VERSION < 14) { + if (NODE_VERSION < 16) { done(); return; } @@ -78,7 +78,7 @@ describe('should report ANR when event loop blocked', () => { }); test('With session', done => { - if (NODE_VERSION < 14) { + if (NODE_VERSION < 16) { done(); return; } @@ -106,7 +106,7 @@ describe('should report ANR when event loop blocked', () => { }); test('from forked process', done => { - if (NODE_VERSION < 14) { + if (NODE_VERSION < 16) { done(); return; } diff --git a/packages/node/src/integrations/anr/index.ts b/packages/node/src/integrations/anr/index.ts index c6919b22e6d8..2753a71bc6f4 100644 --- a/packages/node/src/integrations/anr/index.ts +++ b/packages/node/src/integrations/anr/index.ts @@ -84,8 +84,8 @@ export class Anr implements Integration { /** @inheritdoc */ public setup(client: NodeClient): void { - if ((NODE_VERSION.major || 0) < 14 || ((NODE_VERSION.major || 0) === 14 && (NODE_VERSION.minor || 0) < 7)) { - throw new Error('ANR detection requires Node 14.7.0 or later'); + if ((NODE_VERSION.major || 0) < 16) { + throw new Error('ANR detection requires Node 16 or later'); } // setImmediate is used to ensure that all other integrations have been setup From d098a536c647c75e3748fe0ec2bea89877e8c19d Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 14 Dec 2023 10:43:53 +0100 Subject: [PATCH 11/19] a few worker simplifications --- packages/node/src/integrations/anr/worker.ts | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/packages/node/src/integrations/anr/worker.ts b/packages/node/src/integrations/anr/worker.ts index 7b466d1cae08..ece8af3e28bf 100644 --- a/packages/node/src/integrations/anr/worker.ts +++ b/packages/node/src/integrations/anr/worker.ts @@ -25,15 +25,13 @@ function log(msg: string): void { } } -function getTransport(): Transport { - const url = getEnvelopeEndpointWithUrlEncodedAuth(options.dsn); - return makeNodeTransport({ - url, - recordDroppedEvent: () => { - // - }, - }); -} +const url = getEnvelopeEndpointWithUrlEncodedAuth(options.dsn); +const transport = makeNodeTransport({ + url, + recordDroppedEvent: () => { + // + }, +}); async function sendAbnormalSession(): Promise { // of we have an existing session passed from the main thread, send it as abnormal @@ -47,7 +45,7 @@ async function sendAbnormalSession(): Promise { } const envelope = createSessionEnvelope(session, options.dsn, options.sdkMetadata); - await getTransport().send(envelope); + await transport.send(envelope); try { // Notify the main process that the session has ended so the session can be cleared from the scope @@ -99,8 +97,6 @@ async function sendAnrEvent(frames?: StackFrame[], traceContext?: TraceContext): } const envelope = createEventEnvelope(event, options.dsn, options.sdkMetadata); - const transport = getTransport(); - await transport.send(envelope); await transport.flush(2000); From 27ebe020fbe22fdb73633bf6a920fe356f213689 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Sun, 17 Dec 2023 11:53:07 +0100 Subject: [PATCH 12/19] PR review comments --- .../suites/anr/basic-session.js | 2 ++ .../suites/anr/basic.js | 2 ++ .../suites/anr/basic.mjs | 2 ++ .../suites/anr/forked.js | 2 ++ .../node-integration-tests/suites/anr/test.ts | 29 +++------------- packages/node/src/integrations/anr/index.ts | 33 +++++-------------- packages/node/src/integrations/anr/worker.ts | 20 +++++------ 7 files changed, 28 insertions(+), 62 deletions(-) diff --git a/packages/node-integration-tests/suites/anr/basic-session.js b/packages/node-integration-tests/suites/anr/basic-session.js index 5d31281c67a4..03c8c94fdadf 100644 --- a/packages/node-integration-tests/suites/anr/basic-session.js +++ b/packages/node-integration-tests/suites/anr/basic-session.js @@ -1,4 +1,5 @@ const crypto = require('crypto'); +const assert = require('assert'); const Sentry = require('@sentry/node'); @@ -18,6 +19,7 @@ function longWork() { const salt = crypto.randomBytes(128).toString('base64'); // eslint-disable-next-line no-unused-vars const hash = crypto.pbkdf2Sync('myPassword', salt, 10000, 512, 'sha512'); + assert.ok(hash); } } diff --git a/packages/node-integration-tests/suites/anr/basic.js b/packages/node-integration-tests/suites/anr/basic.js index 30d00a82d5f1..5e0323e2c6c5 100644 --- a/packages/node-integration-tests/suites/anr/basic.js +++ b/packages/node-integration-tests/suites/anr/basic.js @@ -1,4 +1,5 @@ const crypto = require('crypto'); +const assert = require('assert'); const Sentry = require('@sentry/node'); @@ -19,6 +20,7 @@ function longWork() { const salt = crypto.randomBytes(128).toString('base64'); // eslint-disable-next-line no-unused-vars const hash = crypto.pbkdf2Sync('myPassword', salt, 10000, 512, 'sha512'); + assert.ok(hash); } } diff --git a/packages/node-integration-tests/suites/anr/basic.mjs b/packages/node-integration-tests/suites/anr/basic.mjs index 4e297c94b944..17c8a2d460df 100644 --- a/packages/node-integration-tests/suites/anr/basic.mjs +++ b/packages/node-integration-tests/suites/anr/basic.mjs @@ -1,3 +1,4 @@ +import * as assert from 'assert'; import * as crypto from 'crypto'; import * as Sentry from '@sentry/node'; @@ -19,6 +20,7 @@ function longWork() { const salt = crypto.randomBytes(128).toString('base64'); // eslint-disable-next-line no-unused-vars const hash = crypto.pbkdf2Sync('myPassword', salt, 10000, 512, 'sha512'); + assert.ok(hash); } } diff --git a/packages/node-integration-tests/suites/anr/forked.js b/packages/node-integration-tests/suites/anr/forked.js index 30d00a82d5f1..5e0323e2c6c5 100644 --- a/packages/node-integration-tests/suites/anr/forked.js +++ b/packages/node-integration-tests/suites/anr/forked.js @@ -1,4 +1,5 @@ const crypto = require('crypto'); +const assert = require('assert'); const Sentry = require('@sentry/node'); @@ -19,6 +20,7 @@ function longWork() { const salt = crypto.randomBytes(128).toString('base64'); // eslint-disable-next-line no-unused-vars const hash = crypto.pbkdf2Sync('myPassword', salt, 10000, 512, 'sha512'); + assert.ok(hash); } } diff --git a/packages/node-integration-tests/suites/anr/test.ts b/packages/node-integration-tests/suites/anr/test.ts index a6ef0c4046b4..0808cf5d760e 100644 --- a/packages/node-integration-tests/suites/anr/test.ts +++ b/packages/node-integration-tests/suites/anr/test.ts @@ -2,17 +2,16 @@ import * as childProcess from 'child_process'; import * as path from 'path'; import type { Event } from '@sentry/node'; import type { SerializedSession } from '@sentry/types'; -import { parseSemver } from '@sentry/utils'; - -const NODE_VERSION = parseSemver(process.versions.node).major || 0; +import { conditionalTest } from '../../utils'; /** The output will contain logging so we need to find the line that parses as JSON */ function parseJsonLines(input: string, expected: number): T { const results = input .split('\n') .map(line => { + const trimmed = line.startsWith('[ANR Worker] ') ? line.slice(13) : line; try { - return JSON.parse(line) as T; + return JSON.parse(trimmed) as T; } catch { return undefined; } @@ -24,13 +23,8 @@ function parseJsonLines(input: string, expected: number): T return results; } -describe('should report ANR when event loop blocked', () => { +conditionalTest({ min: 14 })('should report ANR when event loop blocked', () => { test('CJS', done => { - if (NODE_VERSION < 16) { - done(); - return; - } - expect.assertions(9); const testScriptPath = path.resolve(__dirname, 'basic.js'); @@ -54,11 +48,6 @@ describe('should report ANR when event loop blocked', () => { }); test('ESM', done => { - if (NODE_VERSION < 16) { - done(); - return; - } - expect.assertions(7); const testScriptPath = path.resolve(__dirname, 'basic.mjs'); @@ -78,11 +67,6 @@ describe('should report ANR when event loop blocked', () => { }); test('With session', done => { - if (NODE_VERSION < 16) { - done(); - return; - } - expect.assertions(9); const testScriptPath = path.resolve(__dirname, 'basic-session.js'); @@ -106,11 +90,6 @@ describe('should report ANR when event loop blocked', () => { }); test('from forked process', done => { - if (NODE_VERSION < 16) { - done(); - return; - } - expect.assertions(7); const testScriptPath = path.resolve(__dirname, 'forker.js'); diff --git a/packages/node/src/integrations/anr/index.ts b/packages/node/src/integrations/anr/index.ts index 2753a71bc6f4..0a7890638de6 100644 --- a/packages/node/src/integrations/anr/index.ts +++ b/packages/node/src/integrations/anr/index.ts @@ -1,3 +1,4 @@ +// TODO (v8): This import can be removed once we only support Node with global URL import { URL } from 'url'; import { getCurrentHub } from '@sentry/core'; import type { Contexts, Event, EventHint, Integration } from '@sentry/types'; @@ -49,26 +50,6 @@ interface InspectorApi { url: () => string | undefined; } -/** - * Starts the node debugger and returns the inspector url. - * - * When inspector.url() returns undefined, it means the port is already in use so we try the next port. - */ -function startInspector(startPort: number = 9229): string | undefined { - // eslint-disable-next-line @typescript-eslint/no-var-requires - const inspector: InspectorApi = require('inspector'); - let inspectorUrl: string | undefined = undefined; - let port = startPort; - - while (inspectorUrl === undefined && port < startPort + 100) { - inspector.open(port); - inspectorUrl = inspector.url(); - port++; - } - - return inspectorUrl; -} - /** * Starts a thread to detect App Not Responding (ANR) events */ @@ -79,7 +60,7 @@ export class Anr implements Integration { /** @inheritdoc */ public setupOnce(): void { - // + // Do nothing } /** @inheritdoc */ @@ -97,8 +78,6 @@ export class Anr implements Integration { */ private async _startWorker(client: NodeClient): Promise { const contexts = await getContexts(client); - const initOptions = client.getOptions(); - const dsn = client.getDsn(); if (!dsn) { @@ -109,9 +88,11 @@ export class Anr implements Integration { delete contexts.app?.app_memory; delete contexts.device?.free_memory; + const initOptions = client.getOptions(); + const sdkMetadata = client.getSdkMetadata() || {}; if (sdkMetadata.sdk) { - sdkMetadata.sdk.integrations = client.getOptions().integrations.map(i => i.name); + sdkMetadata.sdk.integrations = initOptions.integrations.map(i => i.name); } const options: WorkerStartData = { @@ -128,7 +109,9 @@ export class Anr implements Integration { }; if (options.captureStackTrace) { - startInspector(); + // eslint-disable-next-line @typescript-eslint/no-var-requires + const inspector: InspectorApi = require('inspector'); + inspector.open(0); } const { Worker } = getWorkerThreads(); diff --git a/packages/node/src/integrations/anr/worker.ts b/packages/node/src/integrations/anr/worker.ts index ece8af3e28bf..d42a57659419 100644 --- a/packages/node/src/integrations/anr/worker.ts +++ b/packages/node/src/integrations/anr/worker.ts @@ -5,14 +5,15 @@ import { makeSession, updateSession, } from '@sentry/core'; -import type { Event, Session, StackFrame, TraceContext, Transport } from '@sentry/types'; +import type { Event, Session, StackFrame, TraceContext } from '@sentry/types'; import { callFrameToStackFrame, stripSentryFramesAndReverse, watchdogTimer } from '@sentry/utils'; import { Session as InspectorSession } from 'inspector'; import { parentPort, workerData } from 'worker_threads'; import { makeNodeTransport } from '../../transports'; import type { WorkerStartData } from './common'; -type InspectorSessionNodeV12 = InspectorSession & { connectToMainThread: () => void }; +type VoidFunction = () => void; +type InspectorSessionNodeV12 = InspectorSession & { connectToMainThread: VoidFunction }; const options: WorkerStartData = workerData; let session: Session | undefined; @@ -39,10 +40,7 @@ async function sendAbnormalSession(): Promise { log('Sending abnormal session'); updateSession(session, { status: 'abnormal', abnormal_mechanism: 'anr_foreground' }); - if (options.debug) { - // eslint-disable-next-line no-console - console.log(JSON.stringify(session)); - } + log(JSON.stringify(session)); const envelope = createSessionEnvelope(session, options.dsn, options.sdkMetadata); await transport.send(envelope); @@ -91,10 +89,7 @@ async function sendAnrEvent(frames?: StackFrame[], traceContext?: TraceContext): tags: { 'process.name': 'ANR' }, }; - if (options.debug) { - // eslint-disable-next-line no-console - console.log(JSON.stringify(event)); - } + log(JSON.stringify(event)); const envelope = createEventEnvelope(event, options.dsn, options.sdkMetadata); await transport.send(envelope); @@ -107,7 +102,7 @@ async function sendAnrEvent(frames?: StackFrame[], traceContext?: TraceContext): }, 5_000); } -let debuggerPause: () => void | undefined; +let debuggerPause: VoidFunction | undefined; if (options.captureStackTrace) { log('Connecting to debugger'); @@ -177,7 +172,8 @@ if (options.captureStackTrace) { }; } -function createHrTimer(): { getTimeMs: () => number; reset: () => void } { +function createHrTimer(): { getTimeMs: () => number; reset: VoidFunction } { + // TODO (v8): We can use process.hrtime.bigint() after we drop node v8 let lastPoll = process.hrtime(); return { From 94af9ec322a0ed154d7295e6a52526fd9cf730b9 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 18 Dec 2023 13:44:48 +0100 Subject: [PATCH 13/19] Only test node >= 16 --- packages/node-integration-tests/suites/anr/test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/node-integration-tests/suites/anr/test.ts b/packages/node-integration-tests/suites/anr/test.ts index 0808cf5d760e..8d916735e94c 100644 --- a/packages/node-integration-tests/suites/anr/test.ts +++ b/packages/node-integration-tests/suites/anr/test.ts @@ -23,7 +23,7 @@ function parseJsonLines(input: string, expected: number): T return results; } -conditionalTest({ min: 14 })('should report ANR when event loop blocked', () => { +conditionalTest({ min: 16 })('should report ANR when event loop blocked', () => { test('CJS', done => { expect.assertions(9); From 40f25d23e2af6e436eda6d79979714fde2518311 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 18 Dec 2023 15:23:03 +0100 Subject: [PATCH 14/19] Include and test the old deprecated API --- .../suites/anr/legacy.js | 31 ++++++++++++++++++ .../node-integration-tests/suites/anr/test.ts | 24 ++++++++++++++ packages/node/src/index.ts | 2 ++ packages/node/src/integrations/anr/legacy.ts | 32 +++++++++++++++++++ 4 files changed, 89 insertions(+) create mode 100644 packages/node-integration-tests/suites/anr/legacy.js create mode 100644 packages/node/src/integrations/anr/legacy.ts diff --git a/packages/node-integration-tests/suites/anr/legacy.js b/packages/node-integration-tests/suites/anr/legacy.js new file mode 100644 index 000000000000..f3606e9e0b43 --- /dev/null +++ b/packages/node-integration-tests/suites/anr/legacy.js @@ -0,0 +1,31 @@ +const crypto = require('crypto'); +const assert = require('assert'); + +const Sentry = require('@sentry/node'); + +setTimeout(() => { + process.exit(); +}, 10000); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + debug: true, + autoSessionTracking: false, +}); + +// eslint-disable-next-line deprecation/deprecation +Sentry.enableAnrDetection({captureStackTrace: true, anrThreshold: 200}).then(() => { + function longWork() { + for (let i = 0; i < 100; i++) { + const salt = crypto.randomBytes(128).toString('base64'); + // eslint-disable-next-line no-unused-vars + const hash = crypto.pbkdf2Sync('myPassword', salt, 10000, 512, 'sha512'); + assert.ok(hash); + } + } + + setTimeout(() => { + longWork(); + }, 1000); +}); diff --git a/packages/node-integration-tests/suites/anr/test.ts b/packages/node-integration-tests/suites/anr/test.ts index 8d916735e94c..11eb56e2c82e 100644 --- a/packages/node-integration-tests/suites/anr/test.ts +++ b/packages/node-integration-tests/suites/anr/test.ts @@ -47,6 +47,30 @@ conditionalTest({ min: 16 })('should report ANR when event loop blocked', () => }); }); + test('Legacy API', done => { + // TODO (v8): Remove this old API and this test + expect.assertions(9); + + const testScriptPath = path.resolve(__dirname, 'legacy.js'); + + childProcess.exec(`node ${testScriptPath}`, { encoding: 'utf8' }, (_, stdout) => { + const [event] = parseJsonLines<[Event]>(stdout, 1); + + expect(event.exception?.values?.[0].mechanism).toEqual({ type: 'ANR' }); + expect(event.exception?.values?.[0].type).toEqual('ApplicationNotResponding'); + expect(event.exception?.values?.[0].value).toEqual('Application Not Responding for at least 200 ms'); + expect(event.exception?.values?.[0].stacktrace?.frames?.length).toBeGreaterThan(4); + + expect(event.exception?.values?.[0].stacktrace?.frames?.[2].function).toEqual('?'); + expect(event.exception?.values?.[0].stacktrace?.frames?.[3].function).toEqual('longWork'); + + expect(event.contexts?.trace?.trace_id).toBeDefined(); + expect(event.contexts?.trace?.span_id).toBeDefined(); + + done(); + }); + }); + test('ESM', done => { expect.assertions(7); diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index bd67aa71fdfd..ddf1465a3d02 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -78,6 +78,8 @@ export { defaultIntegrations, init, defaultStackParser, getSentryRelease } from export { addRequestDataToEvent, DEFAULT_USER_INCLUDES, extractRequestData } from '@sentry/utils'; export { deepReadDirSync } from './utils'; export { getModuleFromFilename } from './module'; +// eslint-disable-next-line deprecation/deprecation +export { enableAnrDetection } from './integrations/anr/legacy'; import { Integrations as CoreIntegrations } from '@sentry/core'; diff --git a/packages/node/src/integrations/anr/legacy.ts b/packages/node/src/integrations/anr/legacy.ts new file mode 100644 index 000000000000..1d1ebc3024e3 --- /dev/null +++ b/packages/node/src/integrations/anr/legacy.ts @@ -0,0 +1,32 @@ +import { getClient } from '@sentry/core'; +import { Anr } from '.'; +import type { NodeClient } from '../../client'; + +// TODO (v8): Remove this entire file and the `enableAnrDetection` export + +interface LegacyOptions { + entryScript: string; + pollInterval: number; + anrThreshold: number; + captureStackTrace: boolean; + debug: boolean; +} + +/** + * @deprecated Use the `Anr` integration instead. + * + * ```ts + * import * as Sentry from '@sentry/node'; + * + * Sentry.init({ + * dsn: '__DSN__', + * integrations: [new Sentry.Integrations.Anr({ captureStackTrace: true })], + * }); + * ``` + */ +export function enableAnrDetection(options: Partial): Promise { + const client = getClient() as NodeClient; + const integration = new Anr(options); + integration.setup(client); + return Promise.resolve(); +} From 0c71dccd3bfd2b22772eb6b57bc853bd4b330598 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 18 Dec 2023 15:44:24 +0100 Subject: [PATCH 15/19] Linting --- packages/node-integration-tests/suites/anr/legacy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/node-integration-tests/suites/anr/legacy.js b/packages/node-integration-tests/suites/anr/legacy.js index f3606e9e0b43..46b6e1437b10 100644 --- a/packages/node-integration-tests/suites/anr/legacy.js +++ b/packages/node-integration-tests/suites/anr/legacy.js @@ -15,7 +15,7 @@ Sentry.init({ }); // eslint-disable-next-line deprecation/deprecation -Sentry.enableAnrDetection({captureStackTrace: true, anrThreshold: 200}).then(() => { +Sentry.enableAnrDetection({ captureStackTrace: true, anrThreshold: 200 }).then(() => { function longWork() { for (let i = 0; i < 100; i++) { const salt = crypto.randomBytes(128).toString('base64'); From f0ac67e1e121f0627fea8d70d671bb4f46b35f92 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 19 Dec 2023 18:52:17 +0100 Subject: [PATCH 16/19] remove isAnrChildProcess usage --- packages/node-experimental/src/sdk/init.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/node-experimental/src/sdk/init.ts b/packages/node-experimental/src/sdk/init.ts index e7c6ebf72381..821757a9a246 100644 --- a/packages/node-experimental/src/sdk/init.ts +++ b/packages/node-experimental/src/sdk/init.ts @@ -4,7 +4,6 @@ import { defaultIntegrations as defaultNodeIntegrations, defaultStackParser, getSentryRelease, - isAnrChildProcess, makeNodeTransport, } from '@sentry/node'; import type { Integration } from '@sentry/types'; @@ -113,15 +112,14 @@ function getClientOptions(options: NodeExperimentalOptions): NodeExperimentalCli const release = getRelease(options.release); - // If there is no release, or we are in an ANR child process, we disable autoSessionTracking by default const autoSessionTracking = - typeof release !== 'string' || isAnrChildProcess() + typeof release !== 'string' ? false : options.autoSessionTracking === undefined ? true : options.autoSessionTracking; - // We enforce tracesSampleRate = 0 in ANR child processes - const tracesSampleRate = isAnrChildProcess() ? 0 : getTracesSampleRate(options.tracesSampleRate); + + const tracesSampleRate = getTracesSampleRate(options.tracesSampleRate); const baseOptions = dropUndefinedKeys({ transport: makeNodeTransport, From 41cc61d51c13190e9e9ea81859455e7ea55cb491 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 19 Dec 2023 19:10:25 +0100 Subject: [PATCH 17/19] Include `parentSpanId` in trace context --- packages/node/src/integrations/anr/worker.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/node/src/integrations/anr/worker.ts b/packages/node/src/integrations/anr/worker.ts index d42a57659419..fd333314e3eb 100644 --- a/packages/node/src/integrations/anr/worker.ts +++ b/packages/node/src/integrations/anr/worker.ts @@ -139,18 +139,19 @@ if (options.captureStackTrace) { 'Runtime.evaluate', { // Grab the trace context from the current scope - expression: 'const ctx = __SENTRY__.hub.getScope().getPropagationContext(); ctx.traceId + "-" + ctx.spanId', + expression: + 'const ctx = __SENTRY__.hub.getScope().getPropagationContext(); ctx.traceId + "-" + ctx.spanId + "-" + ctx.parentSpanId', // Don't re-trigger the debugger if this causes an error silent: true, }, (_, param) => { - const traceId = param && param.result ? (param.result.value as string) : '-'; - const [trace_id, span_id] = traceId.split('-'); + const traceId = param && param.result ? (param.result.value as string) : '--'; + const [trace_id, span_id, parent_span_id] = traceId.split('-') as (string | undefined)[]; session.post('Debugger.resume'); session.post('Debugger.disable'); - const context = trace_id && span_id ? { trace_id, span_id } : undefined; + const context = trace_id?.length && span_id?.length ? { trace_id, span_id, parent_span_id } : undefined; void sendAnrEvent(stackFrames, context); }, ); From 4a7c4fcad9ab913498cb87a1422799dae341dd15 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 19 Dec 2023 19:16:17 +0100 Subject: [PATCH 18/19] Ensure we get app/device/os/culture context with ANR events --- packages/node-integration-tests/suites/anr/test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/node-integration-tests/suites/anr/test.ts b/packages/node-integration-tests/suites/anr/test.ts index 11eb56e2c82e..f6c7ad521a0d 100644 --- a/packages/node-integration-tests/suites/anr/test.ts +++ b/packages/node-integration-tests/suites/anr/test.ts @@ -43,6 +43,11 @@ conditionalTest({ min: 16 })('should report ANR when event loop blocked', () => expect(event.contexts?.trace?.trace_id).toBeDefined(); expect(event.contexts?.trace?.span_id).toBeDefined(); + expect(event.contexts?.device?.arch).toBeDefined(); + expect(event.contexts?.app?.app_start_time).toBeDefined(); + expect(event.contexts?.os?.name).toBeDefined(); + expect(event.contexts?.culture?.timezone).toBeDefined(); + done(); }); }); From 6271c859eca3f9300f5a5ffc30badc69f72978da Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 19 Dec 2023 19:33:02 +0100 Subject: [PATCH 19/19] Fix test --- packages/node-integration-tests/suites/anr/test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/node-integration-tests/suites/anr/test.ts b/packages/node-integration-tests/suites/anr/test.ts index f6c7ad521a0d..a070f611a0ab 100644 --- a/packages/node-integration-tests/suites/anr/test.ts +++ b/packages/node-integration-tests/suites/anr/test.ts @@ -25,7 +25,7 @@ function parseJsonLines(input: string, expected: number): T conditionalTest({ min: 16 })('should report ANR when event loop blocked', () => { test('CJS', done => { - expect.assertions(9); + expect.assertions(13); const testScriptPath = path.resolve(__dirname, 'basic.js');