From 31b0cdee9d0aa634993364f93995cbfc62f52b15 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 4 Apr 2024 10:52:58 -0400 Subject: [PATCH 1/3] fix(core): unref timer to not block node exit --- packages/core/src/metrics/aggregator.ts | 2 +- packages/core/src/sessionflusher.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/core/src/metrics/aggregator.ts b/packages/core/src/metrics/aggregator.ts index 169e40b42905..f8cca3aa3dcb 100644 --- a/packages/core/src/metrics/aggregator.ts +++ b/packages/core/src/metrics/aggregator.ts @@ -37,7 +37,7 @@ export class MetricsAggregator implements MetricsAggregatorBase { public constructor(private readonly _client: Client) { this._buckets = new Map(); this._bucketsTotalWeight = 0; - this._interval = setInterval(() => this._flush(), DEFAULT_FLUSH_INTERVAL); + this._interval = setInterval(() => this._flush(), DEFAULT_FLUSH_INTERVAL).unref(); this._flushShift = Math.floor((Math.random() * DEFAULT_FLUSH_INTERVAL) / 1000); this._forceFlush = false; } diff --git a/packages/core/src/sessionflusher.ts b/packages/core/src/sessionflusher.ts index 604a654a6b01..aca1686dca73 100644 --- a/packages/core/src/sessionflusher.ts +++ b/packages/core/src/sessionflusher.ts @@ -30,8 +30,8 @@ export class SessionFlusher implements SessionFlusherLike { this._pendingAggregates = {}; this._isEnabled = true; - // Call to setInterval, so that flush is called every 60 seconds - this._intervalId = setInterval(() => this.flush(), this.flushTimeout * 1000); + // Call to setInterval, so that flush is called every 60 seconds. + this._intervalId = setInterval(() => this.flush(), this.flushTimeout * 1000).unref(); this._sessionAttrs = attrs; } From 4abea77a6369cef99d9195fabf23058a3bf54d51 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 4 Apr 2024 11:20:36 -0400 Subject: [PATCH 2/3] add test and add guards for unref --- .../suites/metrics/should-exit-forced.js | 19 +++++++++++++++++ .../suites/metrics/should-exit.js | 18 ++++++++++++++++ .../suites/metrics/test.ts | 21 +++++++++++++++++++ .../tracing/metric-summaries/scenario.js | 3 --- packages/core/src/metrics/aggregator.ts | 5 ++++- packages/core/src/sessionflusher.ts | 5 ++++- 6 files changed, 66 insertions(+), 5 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/metrics/should-exit-forced.js create mode 100644 dev-packages/node-integration-tests/suites/metrics/should-exit.js create mode 100644 dev-packages/node-integration-tests/suites/metrics/test.ts diff --git a/dev-packages/node-integration-tests/suites/metrics/should-exit-forced.js b/dev-packages/node-integration-tests/suites/metrics/should-exit-forced.js new file mode 100644 index 000000000000..2621828973ab --- /dev/null +++ b/dev-packages/node-integration-tests/suites/metrics/should-exit-forced.js @@ -0,0 +1,19 @@ +const Sentry = require('@sentry/node'); + +function configureSentry() { + Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + autoSessionTracking: false, + }); + + Sentry.metrics.increment('test'); +} + +async function main() { + configureSentry(); + await new Promise(resolve => setTimeout(resolve, 1000)); + process.exit(0); +} + +main(); diff --git a/dev-packages/node-integration-tests/suites/metrics/should-exit.js b/dev-packages/node-integration-tests/suites/metrics/should-exit.js new file mode 100644 index 000000000000..01a6f0194507 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/metrics/should-exit.js @@ -0,0 +1,18 @@ +const Sentry = require('@sentry/node'); + +function configureSentry() { + Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + autoSessionTracking: false, + }); + + Sentry.metrics.increment('test'); +} + +async function main() { + configureSentry(); + await new Promise(resolve => setTimeout(resolve, 1000)); +} + +main(); diff --git a/dev-packages/node-integration-tests/suites/metrics/test.ts b/dev-packages/node-integration-tests/suites/metrics/test.ts new file mode 100644 index 000000000000..2c3cc350eeba --- /dev/null +++ b/dev-packages/node-integration-tests/suites/metrics/test.ts @@ -0,0 +1,21 @@ +import { createRunner } from '../../utils/runner'; + +describe('metrics', () => { + test('should exit', done => { + const runner = createRunner(__dirname, 'should-exit.js').start(); + + setTimeout(() => { + expect(runner.childHasExited()).toBe(true); + done(); + }, 5_000); + }); + + test('should exit forced', done => { + const runner = createRunner(__dirname, 'should-exit-forced.js').start(); + + setTimeout(() => { + expect(runner.childHasExited()).toBe(true); + done(); + }, 5_000); + }); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/metric-summaries/scenario.js b/dev-packages/node-integration-tests/suites/tracing/metric-summaries/scenario.js index e2921db180af..422fa4c504a5 100644 --- a/dev-packages/node-integration-tests/suites/tracing/metric-summaries/scenario.js +++ b/dev-packages/node-integration-tests/suites/tracing/metric-summaries/scenario.js @@ -8,9 +8,6 @@ Sentry.init({ transport: loggingTransport, }); -// Stop the process from exiting before the transaction is sent -setInterval(() => {}, 1000); - Sentry.startSpan( { name: 'Test Transaction', diff --git a/packages/core/src/metrics/aggregator.ts b/packages/core/src/metrics/aggregator.ts index f8cca3aa3dcb..2db2ef274063 100644 --- a/packages/core/src/metrics/aggregator.ts +++ b/packages/core/src/metrics/aggregator.ts @@ -37,7 +37,10 @@ export class MetricsAggregator implements MetricsAggregatorBase { public constructor(private readonly _client: Client) { this._buckets = new Map(); this._bucketsTotalWeight = 0; - this._interval = setInterval(() => this._flush(), DEFAULT_FLUSH_INTERVAL).unref(); + this._interval = setInterval(() => this._flush(), DEFAULT_FLUSH_INTERVAL); + if (this._interval.unref) { + this._interval.unref(); + } this._flushShift = Math.floor((Math.random() * DEFAULT_FLUSH_INTERVAL) / 1000); this._forceFlush = false; } diff --git a/packages/core/src/sessionflusher.ts b/packages/core/src/sessionflusher.ts index aca1686dca73..b29f8852e19e 100644 --- a/packages/core/src/sessionflusher.ts +++ b/packages/core/src/sessionflusher.ts @@ -31,7 +31,10 @@ export class SessionFlusher implements SessionFlusherLike { this._isEnabled = true; // Call to setInterval, so that flush is called every 60 seconds. - this._intervalId = setInterval(() => this.flush(), this.flushTimeout * 1000).unref(); + this._intervalId = setInterval(() => this.flush(), this.flushTimeout * 1000); + if (this._intervalId.unref) { + this._intervalId.unref(); + } this._sessionAttrs = attrs; } From e6691424e7830623ebefb3b026b407d9c112de4b Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 4 Apr 2024 12:49:47 -0400 Subject: [PATCH 3/3] Cast to any --- packages/core/src/metrics/aggregator.ts | 9 +++++++-- packages/core/src/sessionflusher.ts | 6 +++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/core/src/metrics/aggregator.ts b/packages/core/src/metrics/aggregator.ts index 2db2ef274063..8b56d190b88a 100644 --- a/packages/core/src/metrics/aggregator.ts +++ b/packages/core/src/metrics/aggregator.ts @@ -20,7 +20,9 @@ export class MetricsAggregator implements MetricsAggregatorBase { // that we store in memory. private _bucketsTotalWeight; - private readonly _interval: ReturnType; + // Cast to any so that it can use Node.js timeout + // eslint-disable-next-line @typescript-eslint/no-explicit-any + private readonly _interval: any; // SDKs are required to shift the flush interval by random() * rollup_in_seconds. // That shift is determined once per startup to create jittering. @@ -37,8 +39,11 @@ export class MetricsAggregator implements MetricsAggregatorBase { public constructor(private readonly _client: Client) { this._buckets = new Map(); this._bucketsTotalWeight = 0; - this._interval = setInterval(() => this._flush(), DEFAULT_FLUSH_INTERVAL); + + this._interval = setInterval(() => this._flush(), DEFAULT_FLUSH_INTERVAL) as any; + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access if (this._interval.unref) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access this._interval.unref(); } this._flushShift = Math.floor((Math.random() * DEFAULT_FLUSH_INTERVAL) / 1000); diff --git a/packages/core/src/sessionflusher.ts b/packages/core/src/sessionflusher.ts index b29f8852e19e..291864333119 100644 --- a/packages/core/src/sessionflusher.ts +++ b/packages/core/src/sessionflusher.ts @@ -20,7 +20,9 @@ export class SessionFlusher implements SessionFlusherLike { public readonly flushTimeout: number; private _pendingAggregates: Record; private _sessionAttrs: ReleaseHealthAttributes; - private _intervalId: ReturnType; + // Cast to any so that it can use Node.js timeout + // eslint-disable-next-line @typescript-eslint/no-explicit-any + private _intervalId: any; private _isEnabled: boolean; private _client: Client; @@ -32,7 +34,9 @@ export class SessionFlusher implements SessionFlusherLike { // Call to setInterval, so that flush is called every 60 seconds. this._intervalId = setInterval(() => this.flush(), this.flushTimeout * 1000); + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access if (this._intervalId.unref) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access this._intervalId.unref(); } this._sessionAttrs = attrs;