False positive pushState warnings when using Sentry

[linsen]

Describe the bug

I'm getting the same false positive warnings as described in #11671 , specifically after installing Sentry using the wizard and default settings (as described in https://docs.sentry.io/platforms/javascript/guides/sveltekit/).

I've attached a minimal repo that's a fresh sveltekit install with Sentry added to it. The warning appears on the first page load after a hard refresh.

Reproduction

https://github.com/linsen/sveltekit-sentry-warning

Logs

Avoid using `history.pushState(...)` and `history.replaceState(...)` as these will conflict with SvelteKit's router. Use the `pushState` and `replaceState` imports from `$app/navigation` instead.

System Info

System:
    OS: macOS 13.3
    CPU: (10) arm64 Apple M1 Pro
    Memory: 73.08 MB / 16.00 GB
    Shell: 3.7.0 - /usr/local/bin/fish
  Binaries:
    Node: 18.18.2 - ~/.nvm/versions/node/v18.18.2/bin/node
    Yarn: 1.22.21 - ~/.nvm/versions/node/v18.18.2/bin/yarn
    npm: 9.8.1 - ~/.nvm/versions/node/v18.18.2/bin/npm
    bun: 1.0.33 - ~/.bun/bin/bun
  Browsers:
    Chrome: 124.0.6367.118
    Safari: 16.4
  npmPackages:
    @sveltejs/adapter-auto: ^3.0.0 => 3.2.0
    @sveltejs/kit: ^2.0.0 => 2.5.7
    @sveltejs/vite-plugin-svelte: ^3.0.0 => 3.1.0
    svelte: ^4.2.7 => 4.2.15
    vite: ^5.0.3 => 5.2.10

Severity

annoyance

Additional Information

No response

[ebeloded]

Dealing with the same issue. Thank you for creating a reproduction!

[Lms24]

Hi Sentry SvelteKit maintainer here 👋 I'll take a look if we can fix this on our end.

As far as I can tell from reading the sveltekit code, this only is a problem in dev mode, though, right?

[Lms24]

Hey, so I took a look and I understand now, why this happens. The reason is indeed that the warning is a false positive. So most importantly: Our SDK does not call history.pushState or history.replaceState. So there's no impact in functionality.

SvelteKit looks at an error stack trace to determine if it should show the warning or not. If you use the Sentry SDK, the SDK will monkey patch history.pushState and history.replaceState, thereby adding one more frame into the stack trace. Again, this doesn't mean we call it, just that we observe when/if it is called for various SDK-internal functionality.

I'll take a look if I can come up with a fix in SvelteKit for this.

[Lms24]

Some more information:

The stack check in SvelteKit slices away a couple of frames because of various stack trace differences in browser (red) and the ones from its own monkey patch (blue):

(this is from chrome browser)

The check would expect line 4 to be line 3, and therefore fails because sentry's monkey patch is in between

IIUC, we should not emit the warning, if lowest replaceState or pushState stack frame originates from the sveltekit client.js file. Cause this means, we called the two functions originally within sveltekit. Does this make sense?

UPDATE: Turns out, it's not so simple to reliably detect the call origin of push|replaceSate calls. Not sure how we can solve this across browsers. For instance, Safari is happy with the current way but FF and Chrome throw the warning.

[dummdidumm]

A possible fix could be to filter out any URLs that have node_modules etc in them. Another (hacky) way could be to have a wrapper that is invoked after a small timeout, which checks how many stack entries it has, and takes that as the base of things to substract (and that way recognizing any monkeyp atches), then removing itself in favor of the dev time check.

[Lms24]

A possible fix could be to filter out any URLs that have node_modules etc in them

Honestly, sounds good enough to me 😅 Are you fine if I submit a PR for this?

[dummdidumm]

Sure, go ahead!