Framework-agnostic Tunnel Handler

[nikolovlazar]

Overview

Right now only the Next.js SDK has a built-in tunnel handler on the backend side, but this PR aims to extract the tunnel handling logic in plain JavaScript in @sentry/core, and provide framework-specific adapters.

Framework-specific adapters checklist

• Next.js
• Remix
• SvelteKit
• Nuxt
• Astro
• Solid Start
• TanStack Start React
• Nest.js
• ...?

Questions for the team

• Is @sentry/core a good home for the framework-agnostic tunnel handler function?
• What framework-specific adapters am I missing in the checklist? (cloudflare, aws, google cloud services, vercel edge, bun, deno)
• How do we standardize this across all SDKs, not just JavaScript?

TODOs

• Finalize framework adapters list
• E2E tests

[github-actions]

node-overhead report 🧳

Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.

Scenario	Requests/s	% of Baseline	Prev. Requests/s	Change %
GET Baseline	8,944	-	9,154	-2%
GET With Sentry	1,776	20%	1,636	+9%
GET With Sentry (error only)	6,158	69%	6,034	+2%
POST Baseline	1,217	-	1,144	+6%
POST With Sentry	611	50%	549	+11%
POST With Sentry (error only)	1,054	87%	1,036	+2%
MYSQL Baseline	3,332	-	3,250	+3%
MYSQL With Sentry	506	15%	333	+52%
MYSQL With Sentry (error only)	2,726	82%	2,646	+3%

View base workflow run

[logaretm]

Is @sentry/core a good home for the framework-agnostic tunnel handler function?

So the issue with @sentry/core is that adding any code to it increases the bundle size across all the SDKs, so this will increase the bundle size of strictly client-side SDKs (e.g: Vue and React) without providing value to their users.

I think having an agnostic tunnel implementation is worth having, but perhaps we should introduce a @sentry/server-utils SDK similiar to the browser-utils one which would contain this logic, and only SSR SDKs can then use it to implement their tunnel utility.

One thing to keep in mind is different implementations of the tunnel feature depends heavily on the framework itself, in Next.js it is implemented as simple re-write rules, so we almost don't have any runtime logic for it on the serve-side.

cc @Lms24

[timfish]

So the issue with @sentry/core is that adding any code to it increases the bundle size across all the SDKs

This is not the case because of tree-shaking. We have the ServerRuntimeClient in @sentry/core and it doesn't impact front-end bundle size.

If a helper like this ends up in the Node SDK it won't be usable in non-Node based SDKs.

[timfish]

I'm not sure it's enough to just check the host matches. We should have a list of allowed DSNs and only forward when they match.

[nikolovlazar]

Yeah the allowedDsnComponents are passed from the outside and they're exactly that - a list of allowed DSNs.

[nikolovlazar]

maybe instead of turning them in components we should pass them as string arrays and do a plain string comparison?

[Lms24]

I kinda like this approach over #14137 because it returns the response to the caller. We need to make sure that the client SDK receives Sentry's response to correctly handle rate limits to avoid backpressure build ups of rate-limited data. This way, the client SDK will apply the rate limits on its end and stop sending stuff to the tunnel endpoint. (Tim, let me know if I misinterpreted your PR and we do in fact handle rate limiting)

I have one request though: Let's split this PR up into the core change that adds the helper and then the Tanstack Start stuff on top. We should also add tests for the core handler alone. I'd recommend we take inspiration from #14137 on the integration test front.

[Lms24]

l: is this really the case? Do we have to create stubs from all of our server exports in TSS? (cc @nicohrubec)

[Lms24]

l: I think we should always assume bytes here, to avoid callers accidentally already strinfifying compressed/binary payloads

Suggested change

	body: string | Uint8Array,
	body: Uint8Array,

[Lms24]

I'd also refactor this a bit to an options object parameter (for future proofing) and as you suggested, make it accept a list of DSN strings. This should be easier for users to use. For us internally, it should make no difference.

[timfish]

This way, the client SDK will apply the rate limits on its end and stop sending stuff to the tunnel endpoint

No you're totally right. This is a better approach!

I think ideally we also want a wrapper with Request in, Response out which will be easier to use for many frameworks.

If we auto setup tunneling to frameworks (like nextjs does) we should use some random ID rather than /sentry or something static because ad-blockers are going to block that too. The nice thing with auto setup is we can include the server-side DSN as allowed.

[Lms24]

I think ideally we also want a wrapper with Request in, Response out which will be easier to use for many frameworks.

Yup that would be ideal. My thinking was that some frameworks use different kinds of Response objects but turns out the one I had in mind (Sveltekit) actually uses the builtin Response. So @nikolovlazar let's start with Request => Response and if necessary create another more abstract handler later.

If we auto setup tunneling to frameworks (like nextjs does) we should use some random ID rather than /sentry or something static because ad-blockers are going to block that too.

+1 for the random id but let's be careful about full auto setup. AFAIK, we don't auto enable tunnelRoute by default in NextJS. The reason being that this can massively increase users' billing.