Merge branch 'develop' into abhi-remove-addNonEnumerableProperty-from-span-utils

Author: AbhiPrasad

packages/core/src/fetch.ts

@@ -1,11 +1,12 @@
+/* eslint-disable complexity */
 import { getClient } from './currentScopes';
 import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from './semanticAttributes';
 import { SPAN_STATUS_ERROR, setHttpStatus, startInactiveSpan } from './tracing';
 import { SentryNonRecordingSpan } from './tracing/sentryNonRecordingSpan';
 import type { FetchBreadcrumbHint, HandlerDataFetch, Span, SpanOrigin } from './types-hoist';
 import { SENTRY_BAGGAGE_KEY_PREFIX } from './utils-hoist/baggage';
 import { isInstanceOf } from './utils-hoist/is';
-import { parseUrl, stripUrlQueryAndFragment } from './utils-hoist/url';
+import { parseStringToURL, stripUrlQueryAndFragment } from './utils-hoist/url';
 import { hasSpansEnabled } from './utils/hasSpansEnabled';
 import { getActiveSpan } from './utils/spanUtils';
 import { getTraceData } from './utils/traceData';
@@ -53,8 +54,20 @@ export function instrumentFetchRequest(
     return undefined;
   }
 
-  const fullUrl = getFullURL(url);
-  const parsedUrl = fullUrl ? parseUrl(fullUrl) : parseUrl(url);
+  // Curious about `thismessage:/`? See: https://www.rfc-editor.org/rfc/rfc2557.html
+  //  > When the methods above do not yield an absolute URI, a base URL
+  //  > of "thismessage:/" MUST be employed. This base URL has been
+  //  > defined for the sole purpose of resolving relative references
+  //  > within a multipart/related structure when no other base URI is
+  //  > specified.
+  //
+  // We need to provide a base URL to `parseStringToURL` because the fetch API gives us a
+  // relative URL sometimes.
+  //
+  // This is the only case where we need to provide a base URL to `parseStringToURL`
+  // because the relative URL is not valid on its own.
+  const parsedUrl = url.startsWith('/') ? parseStringToURL(url, 'thismessage:/') : parseStringToURL(url);
+  const fullUrl = url.startsWith('/') ? undefined : parsedUrl?.href;
 
   const hasParent = !!getActiveSpan();
 
@@ -66,12 +79,13 @@ export function instrumentFetchRequest(
             url,
             type: 'fetch',
             'http.method': method,
-            'http.url': fullUrl,
-            'server.address': parsedUrl?.host,
+            'http.url': parsedUrl?.href,
             [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: spanOrigin,
             [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.client',
-            ...(parsedUrl?.search && { 'http.query': parsedUrl?.search }),
-            ...(parsedUrl?.hash && { 'http.fragment': parsedUrl?.hash }),
+            ...(fullUrl && { 'http.url': fullUrl }),
+            ...(fullUrl && parsedUrl?.host && { 'server.address': parsedUrl.host }),
+            ...(parsedUrl?.search && { 'http.query': parsedUrl.search }),
+            ...(parsedUrl?.hash && { 'http.fragment': parsedUrl.hash }),
           },
         })
       : new SentryNonRecordingSpan();
@@ -215,15 +229,6 @@ function _addTracingHeadersToFetchRequest(
   }
 }
 
-function getFullURL(url: string): string | undefined {
-  try {
-    const parsed = new URL(url);
-    return parsed.href;
-  } catch {
-    return undefined;
-  }
-}
-
 function endSpan(span: Span, handlerData: HandlerDataFetch): void {
   if (handlerData.response) {
     setHttpStatus(span, handlerData.response.status);

packages/core/src/utils-hoist/url.ts

@@ -7,6 +7,33 @@ type PartialURL = {
   hash?: string;
 };
 
+interface URLwithCanParse extends URL {
+  canParse: (url: string, base?: string | URL | undefined) => boolean;
+}
+
+/**
+ * Parses string to a URL object
+ *
+ * @param url - The URL to parse
+ * @returns The parsed URL object or undefined if the URL is invalid
+ */
+export function parseStringToURL(url: string, base?: string | URL | undefined): URL | undefined {
+  try {
+    // Use `canParse` to short-circuit the URL constructor if it's not a valid URL
+    // This is faster than trying to construct the URL and catching the error
+    // Node 20+, Chrome 120+, Firefox 115+, Safari 17+
+    if ('canParse' in URL && !(URL as unknown as URLwithCanParse).canParse(url, base)) {
+      return undefined;
+    }
+
+    return new URL(url, base);
+  } catch {
+    // empty body
+  }
+
+  return undefined;
+}
+
 /**
  * Parses string form of URL into an object
  * // borrowed from https://tools.ietf.org/html/rfc3986#appendix-B

packages/core/test/utils-hoist/url.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, it } from 'vitest';
-import { getSanitizedUrlString, parseUrl, stripUrlQueryAndFragment } from '../../src/utils-hoist/url';
+import { getSanitizedUrlString, parseStringToURL, parseUrl, stripUrlQueryAndFragment } from '../../src/utils-hoist/url';
 
 describe('stripQueryStringAndFragment', () => {
   const urlString = 'http://dogs.are.great:1231/yay/';
@@ -196,3 +196,20 @@ describe('parseUrl', () => {
     expect(parseUrl(url)).toEqual(expected);
   });
 });
+
+describe('parseStringToURL', () => {
+  it('returns undefined for invalid URLs', () => {
+    expect(parseStringToURL('invalid-url')).toBeUndefined();
+  });
+
+  it('returns a URL object for valid URLs', () => {
+    expect(parseStringToURL('https://somedomain.com')).toBeInstanceOf(URL);
+  });
+
+  it('does not throw an error if URl.canParse is not defined', () => {
+    const canParse = (URL as any).canParse;
+    delete (URL as any).canParse;
+    expect(parseStringToURL('https://somedomain.com')).toBeInstanceOf(URL);
+    (URL as any).canParse = canParse;
+  });
+});