"Cannot read properties of undefined (reading 'url')" leads to 405 errors with Next.js 13

SachaG commented 1 year ago

Is there an existing issue for this?

[X] I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
[X] I have reviewed the documentation https://docs.sentry.io/
[X] I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/nextjs

SDK Version

7.55.0

Framework Version

7.55.0

Link to Sentry event

No response

SDK Setup

// This file configures the initialization of Sentry on the server.
// The config you add here will be used whenever the server handles a request.
// https://docs.sentry.io/platforms/javascript/guides/nextjs/

import * as Sentry from '@sentry/nextjs';

Sentry.init({
  dsn: process.env.SENTRY_SERVER_DSN,
  // Adjust this value in production, or use tracesSampler for greater control
  tracesSampleRate: 1.0,
  // ...
  // Note: if you want to override the automatic release value, do not set a
  // `release` value here - use the environment variable `SENTRY_RELEASE`, so
  // that it will also get attached to your source maps
});

Steps to Reproduce

Lately our login API routes (powered by Passport) have been broken, returning an error 405 each time. Here's an example of a route that breaks:

https://github.com/Devographics/Monorepo/blob/main/surveyform/src/pages/api/account/anonymous-login/loginAndCreateResponse.ts

Commenting out the following line in next.config.js fixed the issue, which leads me to believe it might be related to Sentry:

(config) => withSentryConfig(config, sentryWebpackPluginOptions),

I think the problem started when we upgraded from Sentry 6.19.7 to 7.54.0.

Expected Result

No error.

Actual Result

Here is the log with the error:

2023-06-14T07:02:02.334Z    undefined   INFO    - info Loaded env from /var/task/surveyform/.env.production
TypeError: Cannot read properties of undefined (reading 'url')
    at Proxy.handle (/var/task/node_modules/.pnpm/next-connect@0.9.1/node_modules/next-connect/lib/index.js:58:21)
    at /var/task/node_modules/.pnpm/next-connect@0.9.1/node_modules/next-connect/lib/index.js:53:12
    at new Promise (<anonymous>)
    at Proxy.run (/var/task/node_modules/.pnpm/next-connect@0.9.1/node_modules/next-connect/lib/index.js:52:12)
    at Proxy.deprecated (node:internal/util:140:12)
    at Object.apply (/var/task/node_modules/.pnpm/@sentry+nextjs@7.55.0_6y6zewjakh3tibheqprmmqsvhi/node_modules/@sentry/nextjs/cjs/server/wrapApiHandlerWithSentry.js:26:61)
    at /var/task/node_modules/.pnpm/next@13.4.4_vgii64pd2ccbnbx2v3ro5gbin4/node_modules/next/dist/server/api-utils/node.js:463:16
    at /var/task/node_modules/.pnpm/next@13.4.4_vgii64pd2ccbnbx2v3ro5gbin4/node_modules/next/dist/server/lib/trace/tracer.js:117:36
    at NoopContextManager.with (/var/task/node_modules/.pnpm/next@13.4.4_vgii64pd2ccbnbx2v3ro5gbin4/node_modules/next/dist/compiled/@opentelemetry/api/index.js:1:7057)
    at ContextAPI.with (/var/task/node_modules/.pnpm/next@13.4.4_vgii64pd2ccbnbx2v3ro5gbin4/node_modules/next/dist/compiled/@opentelemetry/api/index.js:1:516)

eric-burel commented 1 year ago

Additional info: using @sentry/nextjs v6 + the deprecated apiWrapper in Next.js API routes doesn't produce this issue.

Upgrading to v7 creates the issue.

lforst commented 1 year ago

Hi, there are a lot of moving parts here. Would you mind cleanly setting up Sentry with v7 and removing all the manual wrappers? Thanks.

Also a minimal repro example would help a lot debugging this, since the actual error stems from a different lib than Sentry.

eric-burel commented 1 year ago

Upgrading to v7 + removing apiWrapper still have the same issue indeed, as well as setting automatic tracing to false. We will try to craft a minimal repro later on but for the record the error happens only when Sentry is enabled, it seems to alter the request so the Connect middleware that looks for req.url immediately fails afterwards.

lforst commented 1 year ago

it seems to alter the request so the Connect middleware that looks for req.url immediately fails afterwards.

This is what confuses me a bit. The SDK is very careful in passing everything along. If that weren't the case, I think we would have had a lot more user reports mentioning this.

You can check the code here: https://github.com/getsentry/sentry-javascript/blob/develop/packages/nextjs/src/server/wrapApiHandlerWithSentry.ts

github-actions[bot] commented 1 year ago

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!

"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀