Repository navigation
Conversation
size-limit report 📦Compared: 957bffb (PR test merge using develop at 72b0363) Against: ca4fc32 (develop) · base run #62950
|
19cfdcb to
e1da135
Compare
e1da135 to
8bd81ed
Compare
1404a52 to
45ea3d1
Compare
45ea3d1 to
b227d1c
Compare
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
b227d1c to
808fd83
Compare
|
bugbot run |
808fd83 to
7d51d31
Compare
7d51d31 to
c8efd24
Compare
94ce573 to
3d0c6ae
Compare
3d0c6ae to
c5bc1f3
Compare
c5bc1f3 to
268adff
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 88b440e. Configure here.
88b440e to
c688a7c
Compare
|
One or more gzipped bundles increased by more than 500 bytes. If this increase is intentional, add the Accept Bundlesize Increase label to this PR to rerun and accept the check. |
Optional E2E testsSome optional checks were not successful. They do not block the merge. 1 not successful. See the run.
|
…er build React Router has no request hook to answer from, so `sentryReactRouter()` adds a resource route whose path is read from the DSN at runtime. React Router reads it on its first request, so a request before `Sentry.init` keeps the tunnel off until restart.
The tunnel handler now answers a request without one with a 411.
…drop the variants the base path ones cover
isaacs
left a comment
isaacs
left a comment
There was a problem hiding this comment.
On the right track. Left some comments.
| return { | ||
| code: `${code} | ||
| import { createSentryTunnelRoutes as __sentryCreateTunnelRoutes } from '@sentry/react-router'; | ||
| Object.assign(routes, __sentryCreateTunnelRoutes(typeof basename === 'string' ? basename : undefined)); |
There was a problem hiding this comment.
basename is guarded here, but routes needs to be as well, I think?
| Object.assign(routes, __sentryCreateTunnelRoutes(typeof basename === 'string' ? basename : undefined)); | |
| if (typeof routes === 'object' && routes) { | |
| Object.assign(routes, __sentryCreateTunnelRoutes(typeof basename === 'string' ? basename : undefined)); | |
| } |
| return { 'sentry-tunnel': makeTunnelRoute('sentry-tunnel', getLazyTunnelPath) }; | ||
| } | ||
|
|
||
| return Object.fromEntries( |
There was a problem hiding this comment.
With Sentry.init({ tunnel: '/monitoring' }) and an app route at /monitoring, both routes have equal rank. React Router breaks ties by order, and root's branch comes first. The app route then gets the tunnel POSTs: its action runs (or the request 405s), and the browser falls back or gets errors. TanStack fails the build on that conflict (hasRouteConflict). Here nothing warns. DSN-derived paths can't realistically collide, so this only affects fixed paths.
Suggestion: in createSentryTunnelRoutes, compare each tunnel path against the existing top-level and root-child static paths in routes, and debug.warn on a match (or maybe console.warn, since this is a misconfiguration?) That probably doesn't need to be exact for nested paths. Catching the simple case is enough.
| options.tunnel = applyTunnelDefaults(options.tunnel, { | ||
| basePath: GLOBAL_OBJ_WITH_ROUTER_CONTEXT.__reactRouterContext?.basename, | ||
| }); |
There was a problem hiding this comment.
The browser rewrites an explicit relative tunnel string. This is the same decision as in #25415 and #25419. With basename: '/app/', tunnel: '/tunnel' in the browser now posts to /app/tunnel. Before this PR it posted to /tunnel. A tunnel that lives outside React Router (a proxy or separate service) breaks silently. Here the rewrite happens because applyTunnelDefaults handles relative strings, not because of a framework-specific commit.
Suggestion: settle this once in core (applyTunnelDefaults) for the whole stack. Either leave explicit strings as written, or document the change in the changelog.
React Router has no request hook, so to wire the tunnel logic, we add a resource route to the server build whose path comes from the DSN at runtime.
The base path is the app's
basename, which the server build already exports. Thetunnel-basepathe2e variants cover dev, prod and Cloudflare.For reviewers: if you need an overview of what this is, refer to #25199.
closes #20255