Skip to content

feat(nextjs): Automatically skip middleware requests for tunnel route #16812

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

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions packages/nextjs/src/common/wrapMiddlewareWithSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,31 @@ export function wrapMiddlewareWithSentry<H extends EdgeRouteHandler>(
): (...params: Parameters<H>) => Promise<ReturnType<H>> {
return new Proxy(middleware, {
apply: async (wrappingTarget, thisArg, args: Parameters<H>) => {
const tunnelRoute =
'_sentryRewritesTunnelPath' in globalThis
? (globalThis as Record<string, unknown>)._sentryRewritesTunnelPath
: undefined;

if (tunnelRoute && typeof tunnelRoute === 'string') {
const req: unknown = args[0];
// Check if the current request matches the tunnel route
if (req instanceof Request) {
const url = new URL(req.url);
const isTunnelRequest = url.pathname.startsWith(tunnelRoute);
Comment on lines +39 to +40

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need a url constant since we only use it once and new URL(req.url) is quite explanatory

Suggested change
const url = new URL(req.url);
const isTunnelRequest = url.pathname.startsWith(tunnelRoute);
const isTunnelRequest = new URL(req.url).pathname.startsWith(tunnelRoute);

What do you reckon?


if (isTunnelRequest) {
// Create a simple response that mimics NextResponse.next() so we don't need to import internals here
// which breaks next 13 apps
// https://github.com/vercel/next.js/blob/c12c9c1f78ad384270902f0890dc4cd341408105/packages/next/src/server/web/spec-extension/response.ts#L146
return new Response(null, {
status: 200,
headers: {
'x-middleware-next': '1',
},
}) as ReturnType<H>;
}
}
}
// TODO: We still should add central isolation scope creation for when our build-time instrumentation does not work anymore with turbopack.
return withIsolationScope(isolationScope => {
const req: unknown = args[0];
Expand Down
6 changes: 3 additions & 3 deletions packages/nextjs/src/config/withSentryConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,17 +107,17 @@ function getFinalConfigObject(
showedExportModeTunnelWarning = true;
// eslint-disable-next-line no-console
console.warn(
'[@sentry/nextjs] The Sentry Next.js SDK `tunnelRoute` option will not work in combination with Next.js static exports. The `tunnelRoute` option uses serverside features that cannot be accessed in export mode. If you still want to tunnel Sentry events, set up your own tunnel: https://docs.sentry.io/platforms/javascript/troubleshooting/#using-the-tunnel-option',
'[@sentry/nextjs] The Sentry Next.js SDK `tunnelRoute` option will not work in combination with Next.js static exports. The `tunnelRoute` option uses server-side features that cannot be accessed in export mode. If you still want to tunnel Sentry events, set up your own tunnel: https://docs.sentry.io/platforms/javascript/troubleshooting/#using-the-tunnel-option',
);
}
} else {
const resolvedTunnelRoute =
typeof userSentryOptions.tunnelRoute === 'boolean'
typeof userSentryOptions.tunnelRoute === 'boolean' && userSentryOptions.tunnelRoute === true

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could simplify that, since we know it's a boolean once we pass typeof userSentryOptions.tunnelRoute === 'boolean'

Suggested change
typeof userSentryOptions.tunnelRoute === 'boolean' && userSentryOptions.tunnelRoute === true
typeof userSentryOptions.tunnelRoute === 'boolean' && userSentryOptions.tunnelRoute

Perhaps a bit redundant otherwise

? generateRandomTunnelRoute()
: userSentryOptions.tunnelRoute;

// Update the global options object to use the resolved value everywhere
userSentryOptions.tunnelRoute = resolvedTunnelRoute;
userSentryOptions.tunnelRoute = resolvedTunnelRoute || undefined;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that! 👍

Is there a reason why we want the value to be undefined specifically?

setUpTunnelRewriteRules(incomingUserNextConfigObject, resolvedTunnelRoute);
}
}
Expand Down
106 changes: 105 additions & 1 deletion packages/nextjs/test/config/wrappers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ import type { Client } from '@sentry/core';
import * as SentryCore from '@sentry/core';
import type { IncomingMessage, ServerResponse } from 'http';
import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest';
import { wrapGetInitialPropsWithSentry, wrapGetServerSidePropsWithSentry } from '../../src/common';
import {
wrapGetInitialPropsWithSentry,
wrapGetServerSidePropsWithSentry,
wrapMiddlewareWithSentry,
} from '../../src/common';
import type { EdgeRouteHandler } from '../../src/edge/types';

const startSpanManualSpy = vi.spyOn(SentryCore, 'startSpanManual');

Expand Down Expand Up @@ -84,3 +89,102 @@ describe('data-fetching function wrappers should not create manual spans', () =>
expect(mockSetAttribute).not.toHaveBeenCalled();
});
});

describe('wrapMiddlewareWithSentry', () => {
afterEach(() => {
vi.clearAllMocks();
if ('_sentryRewritesTunnelPath' in globalThis) {
delete (globalThis as any)._sentryRewritesTunnelPath;
}
});

test('should skip processing and return NextResponse.next() for tunnel route requests', async () => {
// Set up tunnel route in global
(globalThis as any)._sentryRewritesTunnelPath = '/monitoring/tunnel';

const origFunction: EdgeRouteHandler = vi.fn(async () => ({ status: 200 }));
const wrappedOriginal = wrapMiddlewareWithSentry(origFunction);

// Create a mock Request that matches the tunnel route
const mockRequest = new Request('https://example.com/monitoring/tunnel?o=123');

const result = await wrappedOriginal(mockRequest);

// Should skip calling the original function
expect(origFunction).not.toHaveBeenCalled();
expect(result).toBeDefined();
});

test('should process normal request and call original function', async () => {
const mockReturnValue = { status: 200 };
const origFunction: EdgeRouteHandler = vi.fn(async (..._args) => mockReturnValue);
const wrappedOriginal = wrapMiddlewareWithSentry(origFunction);

const mockRequest = new Request('https://example.com/api/users', { method: 'GET' });

const result = await wrappedOriginal(mockRequest);

expect(origFunction).toHaveBeenCalledWith(mockRequest);
expect(result).toBe(mockReturnValue);
});

test('should handle non-Request arguments', async () => {
const mockReturnValue = { status: 200 };
const origFunction: EdgeRouteHandler = vi.fn(async (..._args) => mockReturnValue);
const wrappedOriginal = wrapMiddlewareWithSentry(origFunction);

const mockArgs = { someProperty: 'value' };

const result = await wrappedOriginal(mockArgs);

expect(origFunction).toHaveBeenCalledWith(mockArgs);
expect(result).toBe(mockReturnValue);
});

test('should handle errors in middleware function', async () => {
const testError = new Error('Test middleware error');
const origFunction: EdgeRouteHandler = vi.fn(async (..._args) => {
throw testError;
});
const wrappedOriginal = wrapMiddlewareWithSentry(origFunction);

const mockRequest = new Request('https://example.com/api/users');

await expect(wrappedOriginal(mockRequest)).rejects.toThrow('Test middleware error');
expect(origFunction).toHaveBeenCalledWith(mockRequest);
});

test('should not process tunnel route when no tunnel path is set', async () => {
if ('_sentryRewritesTunnelPath' in globalThis) {
delete (globalThis as any)._sentryRewritesTunnelPath;
}

const mockReturnValue = { status: 200 };
const origFunction: EdgeRouteHandler = vi.fn(async (..._args) => mockReturnValue);
const wrappedOriginal = wrapMiddlewareWithSentry(origFunction);

const mockRequest = new Request('https://example.com/monitoring/tunnel/sentry?o=123');

const result = await wrappedOriginal(mockRequest);

// Should process normally since no tunnel path is configured
expect(origFunction).toHaveBeenCalledWith(mockRequest);
expect(result).toBe(mockReturnValue);
});

test('should process request when tunnel path is set but request does not match', async () => {
(globalThis as any)._sentryRewritesTunnelPath = '/monitoring/tunnel';

const mockReturnValue = { status: 200 };
const origFunction: EdgeRouteHandler = vi.fn(async (..._args) => mockReturnValue);
const wrappedOriginal = wrapMiddlewareWithSentry(origFunction);

const mockRequest = new Request('https://example.com/api/users', { method: 'GET' });

const result = await wrappedOriginal(mockRequest);

// Should process normally since request doesn't match tunnel path
expect(origFunction).toHaveBeenCalledWith(mockRequest);
expect(result).toBe(mockReturnValue);
});
});
Loading