8000 feat(core): Add `parseStringToURL` method (#15768) · kevleyski/sentry-javascript@ba5993c · GitHub
[go: up one dir, main page]
Skip to content

Commit ba5993c

Browse files
AbhiPrasadAbhiPrasad
authored
feat(core): Add parseStringToURL method (getsentry#15768)
Now that we support ES2020 (aka not IE11 anymore) and Node.js 18+, we can get rid of `parseUrl` in favor of a method that just uses the built-in URL object. This will save us some bundle size (given we can remove that regex), and we get performance benefits from using native code. Instead of just blanket replacing `parseUrl`, we'll slowly convert all it's usages to using a new helper, `parseStringToURL`. Given we are updating the core package, I also went ahead and converted `parseUrl` usage in `packages/core/src/fetch.ts`
1 parent e55b8ee commit ba5993c

File tree

3 files changed

+66
-17
lines changed

3 files changed

+66
-17
lines changed

‎packages/core/src/fetch.ts

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1+
/* eslint-disable complexity */
12
import { getClient } from './currentScopes';
23
import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from './semanticAttributes';
34
import { SPAN_STATUS_ERROR, setHttpStatus, startInactiveSpan } from './tracing';
45
import { SentryNonRecordingSpan } from './tracing/sentryNonRecordingSpan';
56
import type { FetchBreadcrumbHint, HandlerDataFetch, Span, SpanOrigin } from './types-hoist';
67
import { SENTRY_BAGGAGE_KEY_PREFIX } from './utils-hoist/baggage';
78
import { isInstanceOf } from './utils-hoist/is';
8-
import { parseUrl, stripUrlQueryAndFragment } from './utils-hoist/url';
9+
import { parseStringToURL, stripUrlQueryAndFragment } from './utils-hoist/url';
910
import { hasSpansEnabled } from './utils/hasSpansEnabled';
1011
import { getActiveSpan } from './utils/spanUtils';
1112
import { getTraceData } from './utils/traceData';
@@ -53,8 +54,20 @@ export function instrumentFetchRequest(
5354
return undefined;
5455
}
5556

56-
const fullUrl = getFullURL(url);
57-
const parsedUrl = fullUrl ? parseUrl(fullUrl) : parseUrl(url);
57+
// Curious about `thismessage:/`? See: https://www.rfc-editor.org/rfc/rfc2557.html
58+
// > When the methods above do not yield an absolute URI, a base URL
59+
// > of "thismessage:/" MUST be employed. This base URL has been
60+
// > defined for the sole purpose of resolving relative references
61+
// > within a multipart/related structure when no other base URI is
62+
// > specified.
63+
//
64+
// We need to provide a base URL to `parseStringToURL` because the fetch API gives us a
65+
// relative URL sometimes.
66+
//
67+
// This is the only case where we need to provide a base URL to `parseStringToURL`
68+
// because the relative URL is not valid on its own.
69+
const parsedUrl = url.startsWith('/') ? parseStringToURL(url, 'thismessage:/') : parseStringToURL(url);
70+
const fullUrl = url.startsWith('/') ? undefined : parsedUrl?.href;
5871

5972
const hasParent = !!getActiveSpan();
6073

@@ -66,12 +79,13 @@ export function instrumentFetchRequest(
6679
url,
6780
type: 'fetch',
6881
'http.method': method,
69-
'http.url': fullUrl,
70-
'server.address': parsedUrl?.host,
82+
'http.url': parsedUrl?.href,
7183
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: spanOrigin,
7284
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.client',
73-
...(parsedUrl?.search && { 'http.query': parsedUrl?.search }),< 8000 /div>
74-
...(parsedUrl?.hash && { 'http.fragment': parsedUrl?.hash }),
85+
...(fullUrl && { 'http.url': fullUrl }),
86+
...(fullUrl && parsedUrl?.host && { 'server.address': parsedUrl.host }),
87+
...(parsedUrl?.search && { 'http.query': parsedUrl.search }),
88+
...(parsedUrl?.hash && { 'http.fragment': parsedUrl.hash }),
7589
},
7690
})
7791
: new SentryNonRecordingSpan();
@@ -215,15 +229,6 @@ function _addTracingHeadersToFetchRequest(
215229
}
216230
}
217231

218-
function getFullURL(url: string): string | undefined {
219-
try {
220-
const parsed = new URL(url);
221-
return parsed.href;
222-
} catch {
223-
return undefined;
224-
}
225-
}
226-
227232
function endSpan(span: Span, handlerData: HandlerDataFetch): void {
228233
if (handlerData.response) {
229234
setHttpStatus(span, handlerData.response.status);

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

Lines changed: 27 additions & 0 deletions
< 6D40 td data-grid-cell-id="diff-d16121d32fd1e669a733b622752843df609cc83f67323fc288eb3a1d38827cf5-9-20-1" data-selected="false" role="gridcell" style="background-color:var(--diffBlob-additionNum-bgColor, var(--diffBlob-addition-bgColor-num));text-align:center" tabindex="-1" valign="top" class="focusable-grid-cell diff-line-number position-relative left-side">20
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,33 @@ type PartialURL = {
77
hash?: string;
88
};
99

10+
interface URLwithCanParse extends URL {
11+
canParse: (url: string, base?: string | URL | undefined) => boolean;
12+
}
13+
14+
/**
15+
* Parses string to a URL object
16+
*
17+
* @param url - The URL to parse
18+
* @returns The parsed URL object or undefined if the URL is invalid
19+
*/
+
export function parseStringToURL(url: string, base?: string | URL | undefined): URL | undefined {
21+
try {
22+
// Use `canParse` to short-circuit the URL constructor if it's not a valid URL
23+
// This is faster than trying to construct the URL and catching the error
24+
// Node 20+, Chrome 120+, Firefox 115+, Safari 17+
25+
if ('canParse' in URL && !(URL as unknown as URLwithCanParse).canParse(url, base)) {
26+
return undefined;
27+
}
28+
29+
return new URL(url, base);
30+
} catch {
31+
// empty body
32+
}
33+
34+
return undefined;
35+
}
36+
1037
/**
1138
* Parses string form of URL into an object
1239
* // borrowed from https://tools.ietf.org/html/rfc3986#appendix-B

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, it } from 'vitest';
2-
import { getSanitizedUrlString, parseUrl, stripUrlQueryAndFragment } from '../../src/utils-hoist/url';
2+
import { getSanitizedUrlString, parseStringToURL, parseUrl, stripUrlQueryAndFragment } from '../../src/utils-hoist/url';
33

44
describe('stripQueryStringAndFragment', () => {
55
const urlString = 'http://dogs.are.great:1231/yay/';
@@ -196,3 +196,20 @@ describe('parseUrl', () => {
196196
expect(parseUrl(url)).toEqual(expected);
197197
});
198198
});
199+
200+
describe('parseStringToURL', () => {
201+
it('returns undefined for invalid URLs', () => {
202+
expect(parseStringToURL('invalid-url')).toBeUndefined();
203+
});
204+
205+
it('returns a URL object for valid URLs', () => {
206+
expect(parseStringToURL('https://somedomain.com')).toBeInstanceOf(URL);
207+
});
208+
209+
it('does not throw an error if URl.canParse is not defined', () => {
210+
const canParse = (URL as any).canParse;
211+
delete (URL as any).canParse;
212+
expect(parseStringToURL('https://somedomain.com')).toBeInstanceOf(URL);
213+
(URL as any).canParse = canParse;
214+
});
215+
});

0 commit comments

Comments
 (0)
0