fix(node): Guard against invalid maxSpanWaitDuration values (#14632)

Today, if you pass e.g. `Infinity` or another large number to
`maxSpanWaitDuration`, the SDK will error out because we try to do `new
Array(maxSpanWaitDuration)`, which is impossible.

Ideally, we can properly allow an infinite wait time, but for now this
adds checks to ensure only valid values are passed in there, and we do
not explode.

See
#14154 (comment)

Author: mydea

packages/node/src/sdk/initOtel.ts

@@ -10,6 +10,7 @@ import {
 import { GLOBAL_OBJ, SDK_VERSION, consoleSandbox, logger } from '@sentry/core';
 import { SentryPropagator, SentrySampler, SentrySpanProcessor } from '@sentry/opentelemetry';
 import { createAddHookMessageChannel } from 'import-in-the-middle';
+import { DEBUG_BUILD } from '../debug-build';
 import { getOpenTelemetryInstrumentationToPreload } from '../integrations/tracing';
 import { SentryContextManager } from '../otel/contextManager';
 import type { EsmLoaderHookOptions } from '../types';
@@ -18,6 +19,9 @@ import type { NodeClient } from './client';
 
 declare const __IMPORT_META_URL_REPLACEMENT__: string;
 
+// About 277h - this must fit into new Array(len)!
+const MAX_MAX_SPAN_WAIT_DURATION = 1_000_000;
+
 /**
  * Initialize OpenTelemetry for Node.
  */
@@ -138,7 +142,7 @@ export function setupOtel(client: NodeClient): BasicTracerProvider {
     forceFlushTimeoutMillis: 500,
     spanProcessors: [
       new SentrySpanProcessor({
-        timeout: client.getOptions().maxSpanWaitDuration,
+        timeout: _clampSpanProcessorTimeout(client.getOptions().maxSpanWaitDuration),
       }),
     ],
   });
@@ -152,6 +156,26 @@ export function setupOtel(client: NodeClient): BasicTracerProvider {
   return provider;
 }
 
+/** Just exported for tests. */
+export function _clampSpanProcessorTimeout(maxSpanWaitDuration: number | undefined): number | undefined {
+  if (maxSpanWaitDuration == null) {
+    return undefined;
+  }
+
+  // We guard for a max. value here, because we create an array with this length
+  // So if this value is too large, this would fail
+  if (maxSpanWaitDuration > MAX_MAX_SPAN_WAIT_DURATION) {
+    DEBUG_BUILD &&
+      logger.warn(`\`maxSpanWaitDuration\` is too high, using the maximum value of ${MAX_MAX_SPAN_WAIT_DURATION}`);
+    return MAX_MAX_SPAN_WAIT_DURATION;
+  } else if (maxSpanWaitDuration <= 0 || Number.isNaN(maxSpanWaitDuration)) {
+    DEBUG_BUILD && logger.warn('`maxSpanWaitDuration` must be a positive number, using default value instead.');
+    return undefined;
+  }
+
+  return maxSpanWaitDuration;
+}
+
 /**
  * Setup the OTEL logger to use our own logger.
  */

packages/node/test/sdk/initOtel.test.ts

@@ -0,0 +1,78 @@
+import { logger } from '@sentry/core';
+import { _clampSpanProcessorTimeout } from '../../src/sdk/initOtel';
+
+describe('_clampSpanProcessorTimeout', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('works with undefined', () => {
+    const loggerWarnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
+    const timeout = _clampSpanProcessorTimeout(undefined);
+    expect(timeout).toBe(undefined);
+    expect(loggerWarnSpy).not.toHaveBeenCalled();
+  });
+
+  it('works with positive number', () => {
+    const loggerWarnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
+    const timeout = _clampSpanProcessorTimeout(10);
+    expect(timeout).toBe(10);
+    expect(loggerWarnSpy).not.toHaveBeenCalled();
+  });
+
+  it('works with 0', () => {
+    const loggerWarnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
+    const timeout = _clampSpanProcessorTimeout(0);
+    expect(timeout).toBe(undefined);
+    expect(loggerWarnSpy).toHaveBeenCalledTimes(1);
+    expect(loggerWarnSpy).toHaveBeenCalledWith(
+      '`maxSpanWaitDuration` must be a positive number, using default value instead.',
+    );
+  });
+
+  it('works with negative number', () => {
+    const loggerWarnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
+    const timeout = _clampSpanProcessorTimeout(-10);
+    expect(timeout).toBe(undefined);
+    expect(loggerWarnSpy).toHaveBeenCalledTimes(1);
+    expect(loggerWarnSpy).toHaveBeenCalledWith(
+      '`maxSpanWaitDuration` must be a positive number, using default value instead.',
+    );
+  });
+
+  it('works with -Infinity', () => {
+    const loggerWarnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
+    const timeout = _clampSpanProcessorTimeout(-Infinity);
+    expect(timeout).toBe(undefined);
+    expect(loggerWarnSpy).toHaveBeenCalledTimes(1);
+    expect(loggerWarnSpy).toHaveBeenCalledWith(
+      '`maxSpanWaitDuration` must be a positive number, using default value instead.',
+    );
+  });
+
+  it('works with Infinity', () => {
+    const loggerWarnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
+    const timeout = _clampSpanProcessorTimeout(Infinity);
+    expect(timeout).toBe(1_000_000);
+    expect(loggerWarnSpy).toHaveBeenCalledTimes(1);
+    expect(loggerWarnSpy).toHaveBeenCalledWith('`maxSpanWaitDuration` is too high, using the maximum value of 1000000');
+  });
+
+  it('works with large number', () => {
+    const loggerWarnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
+    const timeout = _clampSpanProcessorTimeout(1_000_000_000);
+    expect(timeout).toBe(1_000_000);
+    expect(loggerWarnSpy).toHaveBeenCalledTimes(1);
+    expect(loggerWarnSpy).toHaveBeenCalledWith('`maxSpanWaitDuration` is too high, using the maximum value of 1000000');
+  });
+
+  it('works with NaN', () => {
+    const loggerWarnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
+    const timeout = _clampSpanProcessorTimeout(NaN);
+    expect(timeout).toBe(undefined);
+    expect(loggerWarnSpy).toHaveBeenCalledTimes(1);
+    expect(loggerWarnSpy).toHaveBeenCalledWith(
+      '`maxSpanWaitDuration` must be a positive number, using default value instead.',
+    );
+  });
+});