Skip to content

Commit 775d609

Browse files
chargomeclaude
andcommitted
fix: Fall back to default interval (not minimum) when collectionIntervalMs is NaN
Previously, non-finite values like NaN would fall back to the 1000ms minimum, which could surprise users expecting the documented default. Distinguish between two error cases: - NaN/Infinity: invalid input → fall back to the integration's default - Below minimum: too low → clamp to 1000ms minimum Update _INTERNAL_normalizeCollectionInterval to accept a defaultInterval parameter and apply it for non-finite inputs. Update deno's inline logic and all tests accordingly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 158783e commit 775d609

File tree

6 files changed

+52
-11
lines changed

6 files changed

+52
-11
lines changed

packages/bun/src/integrations/bunRuntimeMetrics.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ export const bunRuntimeMetricsIntegration = defineIntegration((options: BunRunti
6767
const collectionIntervalMs = _INTERNAL_normalizeCollectionInterval(
6868
options.collectionIntervalMs ?? DEFAULT_INTERVAL_MS,
6969
INTEGRATION_NAME,
70+
DEFAULT_INTERVAL_MS,
7071
);
7172
const collect = {
7273
// Default on

packages/bun/test/integrations/bunRuntimeMetrics.test.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -231,12 +231,20 @@ describe('bunRuntimeMetricsIntegration', () => {
231231
expect(gaugeSpy).toHaveBeenCalled();
232232
});
233233

234-
it('falls back to minimum when NaN', () => {
234+
it('falls back to default when NaN', () => {
235235
const warnSpy = spyOn(globalThis.console, 'warn').mockImplementation(() => {});
236236

237-
bunRuntimeMetricsIntegration({ collectionIntervalMs: NaN });
237+
const integration = bunRuntimeMetricsIntegration({ collectionIntervalMs: NaN });
238+
integration.setup();
238239

239240
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('collectionIntervalMs'));
241+
242+
// Should fire at the default 30000ms, not at 1000ms
243+
jest.advanceTimersByTime(1000);
244+
expect(gaugeSpy).not.toHaveBeenCalled();
245+
246+
jest.advanceTimersByTime(29_000);
247+
expect(gaugeSpy).toHaveBeenCalled();
240248
});
241249
});
242250
});

packages/deno/src/integrations/denoRuntimeMetrics.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,22 @@ export interface DenoRuntimeMetricsOptions {
4949
*/
5050
export const denoRuntimeMetricsIntegration = defineIntegration((options: DenoRuntimeMetricsOptions = {}) => {
5151
const rawInterval = options.collectionIntervalMs ?? DEFAULT_INTERVAL_MS;
52-
if (!Number.isFinite(rawInterval) || rawInterval < MIN_INTERVAL_MS) {
52+
let collectionIntervalMs: number;
53+
if (!Number.isFinite(rawInterval)) {
54+
// eslint-disable-next-line no-console
55+
console.warn(
56+
`[Sentry] denoRuntimeMetricsIntegration: collectionIntervalMs (${rawInterval}) is invalid. Using default of ${DEFAULT_INTERVAL_MS}ms.`,
57+
);
58+
collectionIntervalMs = DEFAULT_INTERVAL_MS;
59+
} else if (rawInterval < MIN_INTERVAL_MS) {
5360
// eslint-disable-next-line no-console
5461
console.warn(
5562
`[Sentry] denoRuntimeMetricsIntegration: collectionIntervalMs (${rawInterval}) is below the minimum of ${MIN_INTERVAL_MS}ms. Using minimum of ${MIN_INTERVAL_MS}ms.`,
5663
);
64+
collectionIntervalMs = MIN_INTERVAL_MS;
65+
} else {
66+
collectionIntervalMs = rawInterval;
5767
}
58-
const collectionIntervalMs = Number.isFinite(rawInterval) ? Math.max(rawInterval, MIN_INTERVAL_MS) : MIN_INTERVAL_MS;
5968
const collect = {
6069
// Default on
6170
memRss: true,

packages/deno/test/deno-runtime-metrics.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ Deno.test('warns and enforces minimum collectionIntervalMs', () => {
135135
assertStringIncludes(warnings[0]!, '1000');
136136
});
137137

138-
Deno.test('warns and falls back to minimum when collectionIntervalMs is NaN', () => {
138+
Deno.test('warns and falls back to default when collectionIntervalMs is NaN', () => {
139139
const warnings: string[] = [];
140140
const originalWarn = globalThis.console.warn;
141141
globalThis.console.warn = (msg: string) => warnings.push(msg);
@@ -148,4 +148,5 @@ Deno.test('warns and falls back to minimum when collectionIntervalMs is NaN', ()
148148

149149
assertEquals(warnings.length, 1);
150150
assertStringIncludes(warnings[0]!, 'collectionIntervalMs');
151+
assertStringIncludes(warnings[0]!, 'invalid');
151152
});

packages/node-core/src/integrations/nodeRuntimeMetrics.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,30 @@ const EVENT_LOOP_DELAY_RESOLUTION_MS = 10;
88

99
/**
1010
* Normalizes a `collectionIntervalMs` value, enforcing a minimum of 1000ms.
11-
* Warns if the value is below the minimum or non-finite (e.g. NaN).
11+
* - Non-finite values (NaN, Infinity): warns and falls back to `defaultInterval`.
12+
* - Values below the minimum: warns and clamps to 1000ms.
1213
* @internal
1314
*/
14-
export function _INTERNAL_normalizeCollectionInterval(rawInterval: number, integrationName: string): number {
15-
if (!Number.isFinite(rawInterval) || rawInterval < MIN_COLLECTION_INTERVAL_MS) {
15+
export function _INTERNAL_normalizeCollectionInterval(
16+
rawInterval: number,
17+
integrationName: string,
18+
defaultInterval: number,
19+
): number {
20+
if (!Number.isFinite(rawInterval)) {
21+
// eslint-disable-next-line no-console
22+
console.warn(
23+
`[Sentry] ${integrationName}: collectionIntervalMs (${rawInterval}) is invalid. Using default of ${defaultInterval}ms.`,
24+
);
25+
return defaultInterval;
26+
}
27+
if (rawInterval < MIN_COLLECTION_INTERVAL_MS) {
1628
// eslint-disable-next-line no-console
1729
console.warn(
1830
`[Sentry] ${integrationName}: collectionIntervalMs (${rawInterval}) is below the minimum of ${MIN_COLLECTION_INTERVAL_MS}ms. Using minimum of ${MIN_COLLECTION_INTERVAL_MS}ms.`,
1931
);
32+
return MIN_COLLECTION_INTERVAL_MS;
2033
}
21-
return Number.isFinite(rawInterval) ? Math.max(rawInterval, MIN_COLLECTION_INTERVAL_MS) : MIN_COLLECTION_INTERVAL_MS;
34+
return rawInterval;
2235
}
2336

2437
export interface NodeRuntimeMetricsOptions {
@@ -83,6 +96,7 @@ export const nodeRuntimeMetricsIntegration = defineIntegration((options: NodeRun
8396
const collectionIntervalMs = _INTERNAL_normalizeCollectionInterval(
8497
options.collectionIntervalMs ?? DEFAULT_INTERVAL_MS,
8598
INTEGRATION_NAME,
99+
DEFAULT_INTERVAL_MS,
86100
);
87101
const collect = {
88102
// Default on

packages/node-core/test/integrations/nodeRuntimeMetrics.test.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -349,13 +349,21 @@ describe('nodeRuntimeMetricsIntegration', () => {
349349
warnSpy.mockRestore();
350350
});
351351

352-
it('falls back to minimum when collectionIntervalMs is NaN', () => {
352+
it('falls back to default when collectionIntervalMs is NaN', () => {
353353
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
354354

355-
nodeRuntimeMetricsIntegration({ collectionIntervalMs: NaN });
355+
const integration = nodeRuntimeMetricsIntegration({ collectionIntervalMs: NaN });
356+
integration.setup();
356357

357358
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('collectionIntervalMs'));
358359

360+
// Should fire at the default 30000ms, not at 1000ms
361+
vi.advanceTimersByTime(1000);
362+
expect(gaugeSpy).not.toHaveBeenCalled();
363+
364+
vi.advanceTimersByTime(29_000);
365+
expect(gaugeSpy).toHaveBeenCalled();
366+
359367
warnSpy.mockRestore();
360368
});
361369
});

0 commit comments

Comments
 (0)