Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): [v7] unref timer to not block node exit #11483

Merged
merged 1 commit into from
Apr 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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