PR feedback

Author: mydea

packages/core/src/tracing/idleSpan.ts

@@ -113,12 +113,25 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
     // Ensure we end with the last span timestamp, if possible
     const spans = getSpanDescendants(span).filter(child => child !== span);
 
-    const latestEndTimestamp = spans.length ? Math.max(...spans.map(span => spanToJSON(span).timestamp || 0)) : 0;
+    // If we have no spans, we just end, nothing else to do here
+    if (!spans.length) {
+      span.end(timestamp);
+      return;
+    }
+
+    const latestEndTimestamp = Math.max(...spans.map(span => spanToJSON(span).timestamp || 0));
+
+    if (latestEndTimestamp <= 0) {
+      span.end(timestamp);
+      return;
+    }
 
     // If the timestamp is smaller than the latest end timestamp of a span, we still want to use it
-    const endTimestamp = latestEndTimestamp
-      ? Math.min(spanTimeInputToSeconds(timestamp), latestEndTimestamp)
-      : timestamp;
+    // Need to convert the timestamp to seconds to ensure Math.min() works as expected
+    // Note that other than this, we can just pass the timestamp as-is to `span.end()`
+    // because that will convert to seconds internally anyhow
+    const spanEndTimestamp = spanTimeInputToSeconds(timestamp);
+    const endTimestamp = Math.min(spanEndTimestamp, latestEndTimestamp);
 
     span.end(endTimestamp);
   }

packages/tracing-internal/src/browser/browserTracingIntegration.ts

@@ -36,23 +36,23 @@ export interface BrowserTracingOptions {
    * The time that has to pass without any span being created.
    * If this time is exceeded, the idle span will finish.
    *
-   * Default: 1000ms
+   * Default: 1000 (ms)
    */
   idleTimeout: number;
 
   /**
    * The max. time an idle span may run.
    * If this time is exceeded, the idle span will finish no matter what.
    *
-   * Default: 30000ms
+   * Default: 30000 (ms)
    */
   finalTimeout: number;
 
   /**
    The max. time an idle span may run.
    * If this time is exceeded, the idle span will finish no matter what.
    *
-   * Default: 15000ms
+   * Default: 15000 (ms)
    */
   childSpanTimeout: number;
 
@@ -185,7 +185,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
   let latestRouteSource: TransactionSource | undefined;
 
   /** Create routing idle transaction. */
-  function _createRouteSpan(client: Client, startSpanOptions: StartSpanOptions): Span | undefined {
+  function _createRouteSpan(client: Client, startSpanOptions: StartSpanOptions): Span {
     const { beforeStartSpan, idleTimeout, finalTimeout, childSpanTimeout } = options;
 
     const isPageloadTransaction = startSpanOptions.op === 'pageload';
@@ -219,10 +219,6 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
       },
     });
 
-    if (!idleSpan) {
-      return;
-    }
-
     if (isPageloadTransaction && WINDOW.document) {
       WINDOW.document.addEventListener('readystatechange', () => {
         if (['interactive', 'complete'].includes(WINDOW.document.readyState)) {