[🐛 Bug]: middleware crashes if sentry installed under src directory

[Enalmada]

next-on-pages environment related information

Happens when deploying to preview but also can be reproduced locally:

System:
Platform: linux
Arch: x64
Version: #1 SMP Fri Jan 27 02:56:13 UTC 2023
CPU: (24) x64 AMD Ryzen 9 3900X 12-Core Processor
Memory: 16 GB
Shell: /bin/bash
Binaries:
Node: 18.16.0
Bun: N/A
pnpm: 8.6.9
Yarn: 1.22.19
npm: 9.8.1
Package Manager Used: pnpm
Relevant Packages:
@cloudflare/next-on-pages: 1.5.0
vercel: 31.2.2
next: 13.4.14-canary.0

Description

When using middleware with an array matcher: ['/blog'] pages immediately crashes with

[mf:inf] HEAD /_next/data/1oj9J4jAV41enV60HBD1V/en/blog.json 500 Internal Server Error (4ms)
TypeError: edgeFunction.default is not a function
    at runOrFetchBuildOutputItem (fh03bwcwbs.js:731:42)
    at async RoutesMatcher.runRouteMiddleware (fh03bwcwbs.js:1006:18)
    at async RoutesMatcher.checkRoute (fh03bwcwbs.js:1127:21)
    at async RoutesMatcher.checkPhase (fh03bwcwbs.js:1169:22)
    at async RoutesMatcher.run (fh03bwcwbs.js:1203:20)
    at async findMatch (fh03bwcwbs.js:1221:18)
    at async handleRequest (fh03bwcwbs.js:1217:17)
    at async jsonError (fh03bwcwbs.js:1363:12) {
  stack: TypeError: edgeFunction.default is not a function
…7)
    at async jsonError (fh03bwcwbs.js:1363:12),
  message: edgeFunction.default is not a function
}

This is valid syntax according to https://nextjs.org/docs/app/building-your-application/routing/middleware#matching-paths

Changing to single string like matcher: '/blog' works fine.

I was trying to deploy

matcher: ['/', '/((?!_next|favicon.ico|api|.*\\.).*)', '/api/login', '/api/logout']

was able to work around it by doing something like this but

matcher: '/$|/((?!_next|favicon\\.ico|api|.*\\..*).*))$|/api/login$|/api/logout$',

Reproduction

Create a middleware file with an matcher containing an array and deploy to next-on-pages:

import { NextResponse } from 'next/server'
import type { NextRequest } from 'next/server'
 
export function middleware(request: NextRequest) {
  return NextResponse.next();
}
 
export const config = {
  matcher: ['/about']
}

Note immediate 500 error. Change to matcher: '/about' and redeploy and everything will work fine.

Pages Deployment Method

Pages CI (GitHub/GitLab integration)

Pages Deployment ID

https://37487ae9.t3-challenge.pages.dev

Additional Information

If you are running minified, the error is:

TypeError: (intermediate value).default is not a function
    at k (xxpe9e4ms1j.js:363:55)
    at async w.runRouteMiddleware (xxpe9e4ms1j.js:515:13)
    at async w.checkRoute (xxpe9e4ms1j.js:554:69)
    at async w.checkPhase (xxpe9e4ms1j.js:579:15)
    at async w.run (xxpe9e4ms1j.js:603:13)
    at async G (xxpe9e4ms1j.js:612:10)
    at async z (xxpe9e4ms1j.js:608:50)
    at async jsonError (xxpe9e4ms1j.js:701:12) {
  stack: TypeError: (intermediate value).default is not a f…0)
    at async jsonError (xxpe9e4ms1j.js:701:12),
  message: (intermediate value).default is not a function
}

Would you like to help?

• Would you like to help fixing this bug?

[dario-piotrowicz]

Hi @Enalmada thanks a bunch for the issue 👍

That's surprising as I've definitely seen this working:

next-on-pages/pages-e2e/features/appMiddleware/assets/middleware.js

Line 89 in 9dbefa1

matcher: ['/api/middleware-test/:path*', '/middleware-test/:path*'],

(maybe it's related to the latest Next canary? 🤔)

I'll have a look 🙂

[dario-piotrowicz]

I did try to use an array matcher in a middleware with @cloudflare/next-on-pages: 1.5.0, vercel: 31.2.2 and next: 13.4.14-canary.0

You can see the code here: middleware app (in particular see the matcher)

The application works as expected (as you can see if for example you go do /test?rewrite), as you can see from this deployment: https://8dcec15b.next-apps-for-testing.pages.dev/

(you can see the build steps here: build workflow)

So I don't really think that just using an array as the matcher actually causes the application to crash, at least not by itself

[dario-piotrowicz]

Even applying your same exact matched doesn't seem to break the application: dario-piotrowicz/next-apps-for-testing@d983fed 😕

See: https://8469ec46.next-apps-for-testing.pages.dev/

---

Could you provide more details about your application? if you could provide a minimal reproduction for the issue that'd be ideal 🙏

[Enalmada]

Thanks @dario-piotrowicz for your time in looking into this. I am on a windows machine using WSL which might have something to do with local reproduction, although that wouldn't explain why just changing that array to string causes the github ci/cd build to deploy something that suddenly works. I will get a reproduction going.

[dario-piotrowicz]

Thanks @dario-piotrowicz for your time in looking into this. I am on a windows machine using WSL which might have something to do with local reproduction, although that wouldn't explain why just changing that array to string causes the github ci/cd build to deploy something that suddenly works. I will get a reproduction going.

No problem 🙂

yeah I am not really sure, maybe in your project there could be a specific combination of things that make that error? 🤷

a reproduction would be really great if you could manage that 🙇

[Enalmada]

After spending some time to get reproduction, I narrowed it down to sentry. If I remove sentry from my app, next-on-pages works. If I just add sentry to the apps/middleware-13.2.4 sample it seems to work ok but moving app and middleware under a src directory then cause the 500 and error above for me. So it seems to be a combination of sentry and src directory.

reproduction:

• Install sentry npx @sentry/wizard@latest -i nextjs. This will create an extra block in next.config.js, some other config files, and some example files. I believe you can take the contents from my reproduction for testing.
• Move app under /src directory. Adjust tsconfig src "paths": { "@/*": ["./src/*"] }
• Add ‘/’ to middleware matcher matcher: ['/', '/api/:path*', '/test/:path*'],

Plot twist... Simple reproduction doesn't seem to matter about matcher.
export const config = {
matcher: '/',
};

Here is my reproduction for reference:
Enalmada/next-apps-for-testing#1

[dario-piotrowicz]

Thanks a lot @Enalmada the reproduction is extremely helpful! 🙏 ❤️

I'm looking into this, I think I understand where the issue lies but I still need to investigate to make sure, and also to find hopefully a solution

I'll update you when there are some news

(PS: sorry for the late reply 🙇)

[dario-piotrowicz]

@Enalmada I've been looking into this and all I could do was improve the error message to be slightly less confusing.

Regarding the actual issue, the problem is that sentry seems to cause the generation of a randomUUID at the top level of the middleware function file.

This wouldn't generally be a problem but it is a problem in the workers runtime because there are some constraints that prevent such type of operations at the top level of dynamically imported files.

Basically for security and performance reasons right now logic at the top level of dynamically imported files doesn't run in the request handling context, and there are limitation as to what can be created outside of such context.

We have been discussing this and trying to find solutions that would be both performant and secure, but it's a bit of a complicated topic and I am not sure if/when the runtime can fix such edge cases.

So unfortunately for now I think there isn't too much we can do about this 😓

[lforst]

@dario-piotrowicz Hi, Sentry SDK maintainer here. Do you know any safe methods we can call to generate IDs? ID generation on the client is at the core of the Sentry architecture so we have the appetite to fix this :D

(EDIT: Ok I had a conversation and apparently we can get around generating an ID and our servers will generate one. We'll take a look what we can do.)

[dario-piotrowicz]

Hi @lforst thank you so so very much for being keen to help out! ❤

If you could generate the id on the server as you mentioned that could fix this, but it might not work and there could be a simpler solution, so let me give you a bit of context here.

The issue with our runtime here is that when we import a dynamic file the imported code is run in a separate context and not the context related to the request handing. Such a context is one that in theory can leak across different requests handling, as such it cannot contain any sort of random or instance related values, it cannot make external http requests also, basically it should only contain sort of global static values that are safe to be reused within different requests handling.

It can contain functions since those when invoked are run inside the current context (which in our case will be the request handling one).

So I don't think that using a different generation method can help, and the server side id generation can help but only as long as that doesn't require an extra fetch run at the exact same time/place as the current id generation.

So as a first step I'd like to ask you if you could check where the id is generated, is the generation done inside some function/method and not at the top level of a file? If it's not inside a function/method could you try moving it into one? (If it already is then the issue could be generated by vercels/our build process so we'd have to find a different solution and/or go with the server side idea)

PS: Thanks so very much again, I'm ultra keen to help however I can to make this work, so please don't hesitate on asking me anything (also unfortunately I'm away this week so I'm going to be a bit limited on how much I can help in the next few days 🙇)

[lforst]

@dario-piotrowicz Thanks for the insight.

I believe our SDK is attempting to generate a UUID outside of a request context / top level to be used as a "baseline" tracing ID. This tracing ID is used for any performance measurements (spans) that are created outside of a request context.

I think we'll just default to basically try-catching the uuid generation and using some non-random dummy ID to give to our users and we'll create a proper one on the Sentry backend.

[dario-piotrowicz]

@lforst I see, that sounds reasonable

I'm sorry you have to jump through these hoops to allow you code to run on our runtime 😔

Thank you so very much again, if you need anything I'm always here 🙂

[dario-piotrowicz]

@lforst I'm back and ready to help/support you in any way I'm able to, did you manage to make any process on your side? is there anything I can help with/clarify? 🙂

[lforst]

@dario-piotrowicz I we released a patch that we think should fix this - but we didn't verify. We're just defaulting to Math.random when crypto is not available. We don't need a reliable source of randomness in our SDK.

[dario-piotrowicz]

Ah ok I see 🙂

yeah I just tested it with @Enalmada's reproduction (where I've updated the @sentry/nextjs version to 7.69.0) and the app seems to be working fine now! 😃 🎉

Thanks a lot @lforst for the hard work! ❤️

@Enalmada please also have a look and let me know if it works in your application 🙂

[lforst]

@dario-piotrowicz Nice thank you so much for verifying! :)

[Enalmada]

I don't see this anymore so closing!

[dario-piotrowicz]

Thanks a lot for confirming this @Enalmada! 😄