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
74c44a9
fec5b0e
9084771
5401817
36b3d81
899a00e
f50fecc
d05d77f
8ea9c69
6c0d9bf
adf78d1
e3b1661
e8da146
2f58c91
b141a6a
c89cc9f
7d6739c
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,18 @@ | ||
| 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.status()).toBe(403); | ||
| }); |
| 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.