Repository navigation
feat(tanstackstart-react): Serve the managed tunnel from the server entry #25408
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
de203a6
5319ce1
90d52a1
b538440
6dfc0e8
8efe375
00e1ebe
21b4dbf
ab59da5
2ca7609
f7e6d22
8a68c00
aef96c6
1a541a3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| import { expect, test } from '@playwright/test'; | ||
|
|
||
| test("keeps the Worker's ExecutionContext when the Sentry Vite plugin wraps the server entry", async ({ request }) => { | ||
| const response = await request.get('/test-execution-context'); | ||
|
|
||
| expect(await response.text()).toBe('function'); | ||
| }); | ||
|
|
||
| test('answers a fixed tunnelRoute with the DSN that `withSentry` initializes', async ({ request }) => { | ||
| // A DSN the server doesn't allow, so the tunnel answers without forwarding anywhere. | ||
| const envelopeHeader = { dsn: 'https://other@o1.ingest.sentry.io/42', sent_at: new Date().toISOString() }; | ||
| const response = await request.post('/monitoring', { | ||
| data: `${JSON.stringify(envelopeHeader)}\n`, | ||
| headers: { 'Content-Type': 'application/x-sentry-envelope' }, | ||
| }); | ||
|
|
||
| expect(response.headers()['x-sentry-tunnel']).toBe('1'); | ||
| expect(response.status()).toBe(403); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,11 @@ | ||
| import { URL_PATH } from '@sentry/conventions/attributes'; | ||
| import type { ServerTunnelOptions } from '@sentry/core'; | ||
| import { dsnToString, escapeStringForRegex, getClient, handleTunnelRequest } from '@sentry/core'; | ||
| import { | ||
| handleTunnelRequestIfMatched, | ||
| _INTERNAL_setManagedTunnelDefaults as setManagedTunnelDefaults, | ||
| } from '@sentry/core/server'; | ||
| import type { ServerEntry } from './wrapFetchWithSentry'; | ||
|
|
||
| const registeredTunnelRoutePaths = new Set<string>(); | ||
|
|
||
|
|
@@ -92,3 +98,34 @@ export function createSentryTunnelRoute(options: CreateSentryTunnelRouteOptions) | |
| }, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Answers managed tunnel requests before the server entry runs. The `sentryTanstackStart` Vite plugin applies this | ||
| * to the app's server entry automatically, with its `tunnelRoute` and the base path Start routes on. | ||
| * | ||
| * `tunnelRoute` enables the managed tunnel on every server client the same way `Sentry.init({ tunnel })` does, unless | ||
| * that `init` already enabled one, and the tunnel is then served from the client's config like in every other SDK. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Recommend marking this |
||
| */ | ||
| export function wrapServerEntryWithSentryTunnel<T extends ServerEntry>( | ||
| entry: T, | ||
| tunnelRoute?: ServerTunnelOptions, | ||
| basePath?: string, | ||
| ): T { | ||
| // Runs when the server entry loads, before any request. Requests still carry Start's base path here, so the tunnel | ||
| // is served under it. | ||
| setManagedTunnelDefaults({ tunnel: tunnelRoute, basePath }); | ||
|
|
||
| // A class entry, like Cloudflare's `WorkerEntrypoint`, can't be spread into an object. The tunnel is then served | ||
| // by the wrapped default server entry it calls. | ||
| if (typeof entry === 'function') { | ||
| return entry; | ||
| } | ||
|
|
||
| return { | ||
| ...entry, | ||
| // Cloudflare Workers pass `env` and `ctx` after the request, and `withSentry` needs `ctx` to flush. | ||
| fetch(request: Request, ...rest: unknown[]) { | ||
| return handleTunnelRequestIfMatched(request) ?? entry.fetch(request, ...rest); | ||
| }, | ||
| }; | ||
|
Comment on lines
+116
to
+130
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggestion: add a short comment that on Cloudflare the inner wrap serves the tunnel because the client is per-request. For the spread, use |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,7 @@ import { makeComposeNitroServiceSourceMapsPlugin, makeEnableNitroSourceMapsPlugi | |
| import { makeRoutePatternPlugin } from './routePatterns'; | ||
| import { makeAddSentryVitePlugin, makeEnableSourceMapsVitePlugin } from './sourceMaps'; | ||
| import type { TunnelRouteOptions } from './tunnelRoute'; | ||
| import { makeTunnelRoutePlugin } from './tunnelRoute'; | ||
| import { makeServerEntryTunnelPlugin, makeTunnelBasePathPlugin, makeTunnelRoutePlugin } from './tunnelRoute'; | ||
|
|
||
| /** | ||
| * Build-time options for the Sentry TanStack Start SDK. | ||
|
|
@@ -37,14 +37,13 @@ export interface SentryTanstackStartOptions extends BuildTimeOptionsBase { | |
| /** | ||
| * Configures a framework-managed same-origin tunnel route for Sentry envelopes. | ||
| * | ||
| * This creates a TanStack Start server route backed by `createSentryTunnelRoute()` and applies the resulting path | ||
| * as the default `tunnel` option on the client. | ||
| * The server entry answers the tunnel before Start's request middleware and routing, and the resulting path is | ||
| * applied as the default `tunnel` option on the client. | ||
| * | ||
| * You can pass: | ||
| * - `true` to generate an opaque route path per dev session or production build. | ||
| * - `true` to serve the path derived from the DSN, the same one `Sentry.init({ tunnel: true })` uses. | ||
| * - `'/custom-path'` to use a fixed static route path. | ||
| * - `{ allowedDsns, path }` for full control. If `allowedDsns` is omitted or empty, the tunnel route derives the DSN | ||
| * from the active server Sentry client at runtime. | ||
| * - `{ allowedDsns, path }` for full control. The DSN of the active server Sentry client is always allowed too. | ||
| * | ||
| * If you also pass `tunnel` to `Sentry.init()`, that explicit runtime option wins and a warning is emitted because | ||
| * the managed tunnel route is being bypassed. | ||
|
|
@@ -77,7 +76,12 @@ export interface SentryTanstackStartOptions extends BuildTimeOptionsBase { | |
| * @returns An array of Vite plugins | ||
| */ | ||
| export function sentryTanstackStart(options: SentryTanstackStartOptions = {}): Plugin[] { | ||
| const plugins: Plugin[] = [makeRoutePatternPlugin()]; | ||
| const tunnelBasePath = makeTunnelBasePathPlugin(); | ||
| const plugins: Plugin[] = [ | ||
| makeRoutePatternPlugin(), | ||
| tunnelBasePath.plugin, | ||
| makeServerEntryTunnelPlugin(options.tunnelRoute, tunnelBasePath.getBasePath), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is always added, so every TanStack Start app that uses
Suggestion: state the tradeoff in the PR body, because the description reads as "only routes matching tunnel requests". We could also consider an opt-out (eg |
||
| ]; | ||
|
|
||
| if (options.tunnelRoute) { | ||
| plugins.push(makeTunnelRoutePlugin(options.tunnelRoute, options.debug)); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, #24615 added the
ANYhandler that returned405 Allow: POST, because without it "a non-POST (or rather GET) request to the route would cause TSS to render a page" (running root loaders). With the wrapper, only POSTs match, so a GET to/monitorfalls through to Start, which SSR-renders the not-found page. The e2e test was changed from expecting405to expecting404, and the test name now describes the new behavior as intended. The PR body does not mention this reversal.Suggestion: either keep the 405 for non-POST requests on a fixed
tunnelRoutepath inwrapServerEntryWithSentryTunnel(the path is known there), or state in the PR body that this reverts #24615 on purpose and why. Note that for the DSN-derived path the old behavior can't hold anyway, since the path is no longer a route, so reverting may well be the right call.