Skip to content

Commit

Permalink
fix(core): unref timer to not block node exit (#11430)
Browse files Browse the repository at this point in the history
Add `unref` to make sure that the session flusher or metrics aggregator
does not block nodejs exit. Especially important for serverless
scenarios.
  • Loading branch information
AbhiPrasad committed Apr 8, 2024
1 parent 9bfadb5 commit 5ab3631
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -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();
18 changes: 18 additions & 0 deletions dev-packages/node-integration-tests/suites/metrics/should-exit.js
Original file line number Diff line number Diff line change
@@ -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();
21 changes: 21 additions & 0 deletions dev-packages/node-integration-tests/suites/metrics/test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@ Sentry.init({
},
});

// Stop the process from exiting before the transaction is sent
setInterval(() => {}, 1000);

Sentry.startSpan(
{
name: 'Test Transaction',
Expand Down
12 changes: 10 additions & 2 deletions packages/core/src/metrics/aggregator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ export class MetricsAggregator implements MetricsAggregatorBase {
// that we store in memory.
private _bucketsTotalWeight;

private readonly _interval: ReturnType<typeof setInterval>;
// 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.
Expand All @@ -42,7 +44,13 @@ export class MetricsAggregator implements MetricsAggregatorBase {
public constructor(private readonly _client: Client<ClientOptions>) {
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);
this._forceFlush = false;
}
Expand Down
11 changes: 9 additions & 2 deletions packages/core/src/sessionflusher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ export class SessionFlusher implements SessionFlusherLike {
public readonly flushTimeout: number;
private _pendingAggregates: Record<number, AggregationCounts>;
private _sessionAttrs: ReleaseHealthAttributes;
private _intervalId: ReturnType<typeof setInterval>;
// 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;

Expand All @@ -30,8 +32,13 @@ export class SessionFlusher implements SessionFlusherLike {
this._pendingAggregates = {};
this._isEnabled = true;

// Call to setInterval, so that flush is called every 60 seconds
// 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;
}

Expand Down

0 comments on commit 5ab3631

Please sign in to comment.