fix(browser): Guard against undefined nextHopProtocol (#16806)

fixes #16804

Although `PerformanceResourceTiming.nextHopProtocol` is available in all
the [browsers we
support](https://docs.sentry.io/platforms/javascript/troubleshooting/supported-browsers/)
in v9, I think adding an extra guard is reasonable to make it easier for
older browser support (it's hard to polyfill some of the performance
apis).

This PR just adds a condition `if (resourceTiming.nextHopProtocol !=
undefined)`.

Used cursor to generate some tests for the
`resourceTimingToSpanAttributes` I extracted out.

Author: AbhiPrasad

packages/browser/src/tracing/request.ts

@@ -2,7 +2,6 @@ import type { Client, HandlerDataXhr, SentryWrappedXMLHttpRequest, Span, WebFetc
 import {
   addFetchEndInstrumentationHandler,
   addFetchInstrumentationHandler,
-  browserPerformanceTimeOrigin,
   getActiveSpan,
   getClient,
   getLocationHref,
@@ -23,10 +22,10 @@ import type { XhrHint } from '@sentry-internal/browser-utils';
 import {
   addPerformanceInstrumentationHandler,
   addXhrInstrumentationHandler,
-  extractNetworkProtocol,
   SENTRY_XHR_DATA_KEY,
 } from '@sentry-internal/browser-utils';
 import { WINDOW } from '../helpers';
+import { resourceTimingToSpanAttributes } from './resource-timing';
 
 /** Options for Request Instrumentation */
 export interface RequestInstrumentationOptions {
@@ -238,8 +237,8 @@ function addHTTPTimings(span: Span): void {
   const cleanup = addPerformanceInstrumentationHandler('resource', ({ entries }) => {
     entries.forEach(entry => {
       if (isPerformanceResourceTiming(entry) && entry.name.endsWith(url)) {
-        const spanData = resourceTimingEntryToSpanData(entry);
-        spanData.forEach(data => span.setAttribute(...data));
+        const spanAttributes = resourceTimingToSpanAttributes(entry);
+        spanAttributes.forEach(attributeArray => span.setAttribute(...attributeArray));
         // In the next tick, clean this handler up
         // We have to wait here because otherwise this cleans itself up before it is fully done
         setTimeout(cleanup);
@@ -248,35 +247,6 @@ function addHTTPTimings(span: Span): void {
   });
 }
 
-function getAbsoluteTime(time: number = 0): number {
-  return ((browserPerformanceTimeOrigin() || performance.timeOrigin) + time) / 1000;
-}
-
-function resourceTimingEntryToSpanData(resourceTiming: PerformanceResourceTiming): [string, string | number][] {
-  const { name, version } = extractNetworkProtocol(resourceTiming.nextHopProtocol);
-
-  const timingSpanData: [string, string | number][] = [];
-
-  timingSpanData.push(['network.protocol.version', version], ['network.protocol.name', name]);
-
-  if (!browserPerformanceTimeOrigin()) {
-    return timingSpanData;
-  }
-  return [
-    ...timingSpanData,
-    ['http.request.redirect_start', getAbsoluteTime(resourceTiming.redirectStart)],
-    ['http.request.fetch_start', getAbsoluteTime(resourceTiming.fetchStart)],
-    ['http.request.domain_lookup_start', getAbsoluteTime(resourceTiming.domainLookupStart)],
-    ['http.request.domain_lookup_end', getAbsoluteTime(resourceTiming.domainLookupEnd)],
-    ['http.request.connect_start', getAbsoluteTime(resourceTiming.connectStart)],
-    ['http.request.secure_connection_start', getAbsoluteTime(resourceTiming.secureConnectionStart)],
-    ['http.request.connection_end', getAbsoluteTime(resourceTiming.connectEnd)],
-    ['http.request.request_start', getAbsoluteTime(resourceTiming.requestStart)],
-    ['http.request.response_start', getAbsoluteTime(resourceTiming.responseStart)],
-    ['http.request.response_end', getAbsoluteTime(resourceTiming.responseEnd)],
-  ];
-}
-
 /**
  * A function that determines whether to attach tracing headers to a request.
  * We only export this function for testing purposes.

packages/browser/src/tracing/resource-timing.ts

@@ -0,0 +1,41 @@
+import type { Span } from '@sentry/core';
+import { browserPerformanceTimeOrigin } from '@sentry/core';
+import { extractNetworkProtocol } from '@sentry-internal/browser-utils';
+
+function getAbsoluteTime(time: number = 0): number {
+  return ((browserPerformanceTimeOrigin() || performance.timeOrigin) + time) / 1000;
+}
+
+/**
+ * Converts a PerformanceResourceTiming entry to span data for the resource span.
+ *
+ * @param resourceTiming
+ * @returns An array where the first element is the attribute name and the second element is the attribute value.
+ */
+export function resourceTimingToSpanAttributes(
+  resourceTiming: PerformanceResourceTiming,
+): Array<Parameters<Span['setAttribute']>> {
+  const timingSpanData: Array<Parameters<Span['setAttribute']>> = [];
+  // Checking for only `undefined` and `null` is intentional because it's
+  // valid for `nextHopProtocol` to be an empty string.
+  if (resourceTiming.nextHopProtocol != undefined) {
+    const { name, version } = extractNetworkProtocol(resourceTiming.nextHopProtocol);
+    timingSpanData.push(['network.protocol.version', version], ['network.protocol.name', name]);
+  }
+  if (!browserPerformanceTimeOrigin()) {
+    return timingSpanData;
+  }
+  return [
+    ...timingSpanData,
+    ['http.request.redirect_start', getAbsoluteTime(resourceTiming.redirectStart)],
+    ['http.request.fetch_start', getAbsoluteTime(resourceTiming.fetchStart)],
+    ['http.request.domain_lookup_start', getAbsoluteTime(resourceTiming.domainLookupStart)],
+    ['http.request.domain_lookup_end', getAbsoluteTime(resourceTiming.domainLookupEnd)],
+    ['http.request.connect_start', getAbsoluteTime(resourceTiming.connectStart)],
+    ['http.request.secure_connection_start', getAbsoluteTime(resourceTiming.secureConnectionStart)],
+    ['http.request.connection_end', getAbsoluteTime(resourceTiming.connectEnd)],
+    ['http.request.request_start', getAbsoluteTime(resourceTiming.requestStart)],
+    ['http.request.response_start', getAbsoluteTime(resourceTiming.responseStart)],
+    ['http.request.response_end', getAbsoluteTime(resourceTiming.responseEnd)],
+  ];
+}