Auto Instrumentation breaks Next.js edge API config (vercel region)

[rookbreezy]

Is there an existing issue for this?

• I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
• I have reviewed the documentation https://docs.sentry.io/
• 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.43.0

SDK Setup

sentry.edge.config

Sentry.init({ dsn })

Steps to Reproduce

Create pages/api/test.ts and build

export const config = { runtime: 'edge', regions: 'iad1' }

export default () => {
  throw new Error('Test')
}

Expected Result

Region should be iad1 (Washington, D.C., USA)

Actual Result

Region is Global (default)

Quick fix

If we set:

excludeServerRoutes: ['/api/test']

And re-deploy, Vercel shows the correct region of Washington, D.C., USA

Debugging

Running next build locally I looked in .next/server/middleware-manifest.json

{
  "/api/test": {
      "env": [
      ],
      "files": [
        "server/edge-runtime-webpack.js",
        "server/pages/api/test.js"
      ],
      "name": "pages/api/test",
      "page": "/api/test",
      "matchers": [
        {
          "regexp": "^/api/test$",
          "originalSource": "/api/test"
        }
      ],
      "wasm": [],
      "assets": []
    }
}

It's missing the regions property. It should look like this:

 "regions": "iad1"

Attempts to fix

I have also tried:

• Named function export
• Named arrow function export
• setting regions to an array ['iad1']

All the same result.

I also tried wrapping the function myself. But I could not get wrapApiHandlerWithSentry to work at all in edge routes, maybe I am doing something wrong. It did work in node routes.

Am I missing something obvious? I am not sure what the root problem is but hopefully this is useful.

[AbhiPrasad]

Hey @coding-camel! We actually had the chance to talk to the Next.js team at Vercel about this.

Vercel actually uses static analysis to read this value. This means any build tool or Webpack loader/plugin that non-trivially modifies code at bundle time will trip it up. This is unfortunately what the Sentry SDK is running into.

This requires Vercel to fix this upstream - I would recommend reaching out to them!

Until then you have to disable the wrapping using excludeServerRoutes unfortunately, since we have no better way around this on the Sentry SDK side.

[rookbreezy]

@AbhiPrasad Why would Sentry need to fiddle with other exports? Shouldn't it just wrap the default export and be done?

Thank you for looking into it.

[AbhiPrasad]

@coding-camel it shouldn't fiddle with any exports - you can see this if you tried to run a route in production and import config from it.

pages/api/test.ts

export const config = { runtime: 'edge', regions: 'iad1' }

export default () => {
  throw new Error('Test')
}

pages/api/test2.ts

import { config } from "./test";

export default () => {
  console.log(config);
  throw new Error('Test')
}

It's also why using the config to set a function as edge still works. We run e2e tests for this:

sentry-javascript/packages/e2e-tests/test-applications/nextjs-app-dir/pages/api/edge-endpoint.ts

Line 1 in d6561b8

export const config = { runtime: 'edge' };

Vercel should be grabbing the regions config the same way they are grabbing the runtime config, but they are not, causing this discrepancy.

If you had decided to use another transform, or a custom swc plugin to build your edge function you would run into the same issue (which other users do).

[AbhiPrasad]

@coding-camel I've reached out to Vercel folks I have a contact with, but I recommend you also do the same if you can. Let's see if we can come up with a resolution for this problem!

[rookbreezy]

@AbhiPrasad Hey, I reached out to them, this is the response I got today.

The team is already looking into this and has been discussing the plan with the Sentry team.

[github-actions]

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 🥀

[lforst]

Ok, let's try to come up with a solution for this. This problem is a bit nuanced and I think I am opinionated on this so I'll try to explain the problem step by step. Maybe I'll change my mind by the end of my explanation :D

What does the Sentry Next.js SDK do?

In order to instrument API routes we built a webpack loader that wraps your routes with error and performance instrumentation. The loader achieves this by importing your route module from a "virtual" module, wrapping your route handler in a Sentry function, and re-exporting everything from the virtual module. Additionally, we are setting the api.externalResolver field in the exported config object to mute Next.js warnings that are emitted because we are patching res.end() to flush error events. The result is then bundled and returned.

Under the hood, we're using Rollup to take care of the bundling. The main reason for this is that rollup is very flexible and battle-tested. We have identified a lot of issues by now that we would have run into if we did manual AST manipulation to wrap the handler.

How does this break the edge region config?

When bundling with Rollup, we are changing how the config export looks - a) by inserting a api.externalResolver value b) syntactically rollup changes how the exports look.

Next.js constructs the build manifest containing the edge region by doing static code analysis on the module AFTER loaders have been applied (TODO link to relevant code section in Next.js repo, maybe Vercel folks can help out here 🙏). IIRC the pattern Next.js looks for is pretty strict so the way we transform the config export actually breaks this analysis.

What possible solutions are there?

There are a few possible routes to fix this, both from Next.js' side and from Sentry's side. Here's a list that is probably not complete:

• We could copy the static code analysis logic from Next.js and adjust our bundled output accordingly.
  • To me, this feels silly and somewhat fragile. When Next.js changes something in their logic, the logic in the SDK breaks again. I would prefer not to go this route.
• Instead of doing static analysis, we evaluate the contents of the config export.
  • I personally like this solution but I don't know the full extent of complications it might come with. Maybe the user code has side effects that become problematic if executed.
  • Theoretically, this could be done on either side - Sentry SDK or Next.js. To me DX-wise it makes more sense to me that this is done directly in Next.js because users might run into this problem regardless of Sentry if they define their config export in a weird way.
• Next.js could run the static code analysis BEFORE loaders/plugins are run.
  • I don't know the inner workings enough if this is feasible in Next.js though.

Solution brainstorm

I see two sides to this problem. First, the Sentry SDK is breaking a feature of Next.js and we need to fix it. The second problem I see is with the static code analysis that Next.js is doing. To me personally, it seems a bit too strict.

I would love your thoughts on this @leerob, @Schniz (maybe also others who can comment). Do you have any other ideas on how we could tackle this?

[lforst]

Hi, I just tested this again and it looks like this resolved itself. I assume the nice folks over at Vercel improved the detection logic for this. To be sure, I still recommend upgrading the SDK to the latest version. Let me know if you still have any problems related to this issue.

[rookbreezy]

@lforst I can confirm edge regions are correct for me now. Sending manual/instrumented errors both work now, great.

The errors are missing stack trace and incoming req info. I think the first issue is tracked here #6805

Nice to see this completed.

[lforst]

The events are missing much info. (compared to node). But that is a separate issue I think. And I am guessing the Sentry team is aware of it?

Yes we are :) Some of this is due to technical limitations but there is a lot we can improve.

[rookbreezy]

Is the coming request info (tags) issue tracked somewhere? Or is the issue on my side?

[lforst]

It's an item here: #6726