Repository navigation
feat(remix): Serve the managed tunnel from the Remix 3 router middleware #25420
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
09673f5
0d0b516
5ba4b66
fe3e798
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,25 @@ | ||
| import { expect, test } from '@playwright/test'; | ||
| import { isServerSpanAt, watchForTunnelSpan } from '@sentry-internal/test-utils'; | ||
|
|
||
| test.skip(!process.env.E2E_TEST_TUNNEL, 'Only runs in the tunnel variant'); | ||
|
|
||
| const TUNNEL_PATH = /^\/[a-z0-9]{1,7}$/; | ||
|
|
||
| test('answers the managed tunnel before the app handler, route matching and the app middleware', async ({ page }) => { | ||
| // The variant's handler rejects every other POST before its router, except with the plain server, where nothing ahead | ||
| // of the router could answer the tunnel. Its auth middleware rejects the POSTs that get through. | ||
| if (!process.env.E2E_TEST_PLAIN_SERVER) { | ||
| expect((await page.request.post('/anything', { data: 'x' })).status()).toBe(403); | ||
| } | ||
| expect((await page.request.post('/anything', { data: 'x', headers: { 'x-app-auth': 'ok' } })).status()).toBe(401); | ||
|
|
||
| const raceTunnelSpan = watchForTunnelSpan('remix-v3', isServerSpanAt(TUNNEL_PATH)); | ||
| const tunnelResponsePromise = page.waitForResponse( | ||
| response => response.request().method() === 'POST' && TUNNEL_PATH.test(new URL(response.url()).pathname), | ||
| ); | ||
|
|
||
| await page.goto('/'); | ||
|
|
||
| expect((await tunnelResponsePromise).status()).toBe(200); | ||
| expect(await raceTunnelSpan(isServerSpanAt('/api/items/1'), () => page.request.get('/api/items/1'))).toBe('anchor'); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ import { | |
| updateSpanName, | ||
| winterCGRequestToRequestData, | ||
| } from '@sentry/core'; | ||
| import { handleTunnelRequestIfMatched } from '@sentry/core/server'; | ||
|
|
||
| import { formatRouteTiming } from '../routeTiming'; | ||
| import type { MatcherLike, MiddlewareLike, NextFunctionLike, RequestContextLike } from '../types'; | ||
|
|
@@ -25,6 +26,11 @@ import { resolveRoutePattern } from './route'; | |
| */ | ||
| export function sentryRemixMiddleware(matcher: MatcherLike): MiddlewareLike { | ||
| return async function sentryMiddleware(context: RequestContextLike, next: NextFunctionLike): Promise<Response> { | ||
| const tunnelResponse = handleTunnelRequestIfMatched(context.request); | ||
| if (tunnelResponse) { | ||
| return tunnelResponse; | ||
| } | ||
|
Comment on lines
+29
to
+32
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. A general idea: you added the tunnel in wdyt?
Member
Author
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. I think it makes sense, currently it didn't cover apps without a router. |
||
|
|
||
| const isolationScope = getIsolationScope(); | ||
| isolationScope.setSDKProcessingMetadata({ normalizedRequest: winterCGRequestToRequestData(context.request) }); | ||
|
|
||
|
|
||
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.
Conditional assertion in tunnel test
Low Severity
Flagged because the review guidelines ask to split a test that uses a conditional for different paths. The
E2E_TEST_PLAIN_SERVERbranch only asserts the pre-router403in one variant, so a failure on that path is easy to miss or misread as a skip.Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 53b7826. Configure here.