getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
8.01k stars 1.58k forks source link

feat(core): Deprecate `parseUrl` #14404

Closed lforst closed 3 hours ago

lforst commented 1 day ago

Ref https://github.com/getsentry/sentry-javascript/issues/14267

Deprecates and removes usages of parseUrl which can be replaced with the URL constructor with our current version support.

github-actions[bot] commented 1 day ago

size-limit report 📦

Path Size % Change Change
@sentry/browser 22.78 KB -0.44% -102 B 🔽
@sentry/browser - with treeshaking flags 21.48 KB -0.44% -96 B 🔽
@sentry/browser (incl. Tracing) 35.41 KB -0.06% -20 B 🔽
@sentry/browser (incl. Tracing, Replay) 72.12 KB +0.02% +9 B 🔺
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 62.4 KB -0.03% -18 B 🔽
@sentry/browser (incl. Tracing, Replay with Canvas) 76.41 KB -0.01% -5 B 🔽
@sentry/browser (incl. Tracing, Replay, Feedback) 88.88 KB -0.02% -10 B 🔽
@sentry/browser (incl. Feedback) 39.51 KB -0.27% -106 B 🔽
@sentry/browser (incl. sendFeedback) 27.41 KB -0.37% -103 B 🔽
@sentry/browser (incl. FeedbackAsync) 32.22 KB -0.3% -98 B 🔽
@sentry/react 25.48 KB -0.41% -105 B 🔽
@sentry/react (incl. Tracing) 38.26 KB -0.07% -25 B 🔽
@sentry/vue 26.93 KB -0.38% -105 B 🔽
@sentry/vue (incl. Tracing) 37.23 KB -0.05% -16 B 🔽
@sentry/svelte 22.91 KB -0.51% -119 B 🔽
CDN Bundle 23.95 KB -0.4% -97 B 🔽
CDN Bundle (incl. Tracing) 36.98 KB -0.03% -11 B 🔽
CDN Bundle (incl. Tracing, Replay) 71.67 KB -0.03% -22 B 🔽
CDN Bundle (incl. Tracing, Replay, Feedback) 77.02 KB -0.04% -28 B 🔽
CDN Bundle - uncompressed 70.72 KB -0.27% -190 B 🔽
CDN Bundle (incl. Tracing) - uncompressed 110.15 KB +0.03% +28 B 🔺
CDN Bundle (incl. Tracing, Replay) - uncompressed 222.68 KB +0.02% +28 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 235.89 KB +0.02% +28 B 🔺
@sentry/nextjs (client) 38.36 KB -0.08% -31 B 🔽
@sentry/sveltekit (client) 35.92 KB -0.05% -16 B 🔽
@sentry/node 134.41 KB -0.06% -74 B 🔽
@sentry/node - without tracing 96.25 KB -0.07% -65 B 🔽
@sentry/aws-serverless 106.5 KB -0.07% -72 B 🔽

View base workflow run

codecov[bot] commented 1 day ago

:x: 17 Tests Failed:

Tests completed Failed Passed Skipped
653 17 636 28
View the top 3 failed tests by shortest run time > > ```python > transactions.test.ts Sends an API route transaction > ``` > >
Stack Traces | 0.021s run time > > > > > ```python > > transactions.test.ts:4:5 Sends an API route transaction > > ``` > >
transactions.test.ts Sends an API route transaction from module
Stack Traces | 0.021s run time > > ```python > transactions.test.ts:4:5 Sends an API route transaction from module > ```
transactions.test.ts Sends successful transaction
Stack Traces | 0.028s run time > > ```python > transactions.test.ts:4:5 Sends successful transaction > ```

To view more test analytics, go to the Test Analytics Dashboard Got feedback? Let us know on Github

mydea commented 1 day ago

IMHO this is fine, but:

  1. let's always try-catch new URL( calls to ensure we do not throw for weird input or so.
  2. Let's also make sure we do not regress anything by not handling relative paths only. Now, we seem to handle it sometimes, but not always.

Generally I wonder if it would not be easier/better to just let the method be and just rewrite it to use new URL under the hood 😅 then we could centralize/streamline both points I mentioned above. I'm also fine with repeating this everywhere we use this, but it does feel a bit boiler-platey to me 🤷

lforst commented 4 hours ago

Generally I wonder if it would not be easier/better to just let the method be and just rewrite it to use new URL under the hood 😅 then we could centralize/streamline both points I mentioned above. I'm also fine with repeating this everywhere we use this, but it does feel a bit boiler-platey to me 🤷

I had the exact same thought today, the only problem I had with it was that it is maybe too generic to be exported as API from a Sentry package. At the same time, nobody probably cares.

lforst commented 3 hours ago

Let's keep this funciton and replace the implementation with URL in a non-breaking way.