Skip to content
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

fix(nextjs): Stop injecting sentry into API middleware #4517

Merged

Conversation

lobsterkatie
Copy link
Member

@lobsterkatie lobsterkatie commented Feb 8, 2022

During the build of a nextjs app, our SDK injects the user's sentry.server.config.js (which contains the call to Sentry.init()) into all API routes, so that withSentry will work. That said, we don't yet support nextjs middleware, so we shouldn't be injecting the file into middleware pages, even if they're in the api/ folder.

This prevents that by adding another condition to shouldAddSentryToEntryPoint.

Note: This came up as part of the issue referenced below, and it does fix it, but only in a temporary way. The underlying issue is (I believe) something outside of sentry. I've encouraged the user to debug it, so that when we start supporting middleware (and therefore remove this filter), he doesn't run into problems again. See this comment.

Fixes #4507.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 8, 2022

size-limit report

Path Base Size (52520ba) Current Size Change
@sentry/browser - CDN Bundle (gzipped) 19.69 KB 19.69 KB +0.01% 🔺
@sentry/browser - CDN Bundle (minified) 62.91 KB 62.91 KB +0.01% 🔺
@sentry/browser - Webpack 22.21 KB 22.21 KB 0%
@sentry/browser - Webpack - gzip = false 76.02 KB 76.02 KB 0%
@sentry/react - Webpack 22.24 KB 22.24 KB 0%
@sentry/nextjs Client - Webpack 46.31 KB 46.31 KB 0%
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped) 28.22 KB 28.23 KB +0.02% 🔺

@lobsterkatie lobsterkatie merged commit fa992bd into master Feb 8, 2022
@lobsterkatie lobsterkatie deleted the kmclb-nextjs-stop-injecting-into-api-middleware branch February 8, 2022 01:56
@@ -262,7 +262,7 @@ function checkWebpackPluginOverrides(
function shouldAddSentryToEntryPoint(entryPointName: string, isServer: boolean): boolean {
return (
entryPointName === 'pages/_app' ||
entryPointName.includes('pages/api') ||
(entryPointName.includes('pages/api') && !entryPointName.includes('_middleware')) ||
Copy link

Choose a reason for hiding this comment

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

It's an unlikely situation, but in theory a user could name its non-middleware fn something like 'pages/api/_middleware/foo.ts', which will match this check and thus inject sentry where it shouldn't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@sentry/nextjs - _middleware.ts inside /pages/api-dir gets sentry-related code in production build
3 participants