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

@sentry/nextjs 7.15 and 7.16 have issues with @vercel/nft #5998

Closed
3 tasks done
kachkaev opened this issue Oct 19, 2022 · 10 comments · Fixed by #6685 · May be fixed by vercel/nft#322
Closed
3 tasks done

@sentry/nextjs 7.15 and 7.16 have issues with @vercel/nft #5998

kachkaev opened this issue Oct 19, 2022 · 10 comments · Fixed by #6685 · May be fixed by vercel/nft#322
Assignees
Labels
Package: nextjs Issues related to the Sentry Nextjs SDK Type: Bug

Comments

@kachkaev
Copy link

kachkaev commented Oct 19, 2022

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which package are you using?

@sentry/nextjs

SDK Version

7.15.0 / 7.16.0

Framework Version

Next 12.3.1

Link to Sentry event

No response

Steps to Reproduce

When upgrading blockprotocol/blockprotocol from @sentry/nextjs@7.13.0 to 7.15.0 or 7.16.0, my app started responding with 500 on Vercel. It was working fine locally, which did not make sense at first. I managed to link this problem to @vercel/nft (part of Next.js) and created an MWE here:
https://github.com/hasharchives/sentry-vercel-nft-issue/branches/all

The bug it is to do with Sentry corrupting *.nft.json build files.

The default no-sentry branch in my MWE contains a simple Next.js app that consists of a page with getServerSideProps and an API route. Both files read data.json in the repo root. When the app is built, Next creates .next/server/pages/**/*.nft.json files which are then used on Vercel to determine which files to include in which lambdas. These files are normally .gitignore-ed but I’ve added them to the MWE repo:

Autogenerated *.nft.json files are one-liners. I ran yarn dlx prettier --write .next/server/pages/**/*.nft.json and then ordered file paths alphabetically to ease comparison.

As you can see, both files above refer data.json which is correct.

Installing @sentry/nextjs@7.13.0 or @sentry/nextjs@7.14.0 results in more items in *.nft.json but does not affect the presence of data.json – that’s good:

However, switching to newer versions of @sentry/nextjs causes data.json to disappear:

Although the result of next build works locally via yarn start, the Vercel deployment ends up corrupt because of the missing references. Lambda logs show this:


ERROR	Error: /var/task/data.json: ENOENT: no such file or directory, open '/var/task/data.json'
    at Object.openSync (node:fs:585:3)
    at Object.readFileSync (node:fs:453:35)
    at Object.readFileSync (/var/task/node_modules/jsonfile/index.js:50:22)
    at Object.9791 (/var/task/.next/server/chunks/9791.js:70:49)
    at __webpack_require__ (/var/task/.next/server/webpack-runtime.js:25:42)
    at /var/task/.next/server/chunks/845.js:1566:78
    at Function.__webpack_require__.a (/var/task/.next/server/webpack-runtime.js:89:13)
    at Object.8841 (/var/task/.next/server/chunks/845.js:1548:21)
    at __webpack_require__ (/var/task/.next/server/webpack-runtime.js:25:42)
    at /var/task/.next/server/chunks/845.js:1508:89 {
  errno: -2,
  syscall: 'open',
  code: 'ENOENT',
  path: '/var/task/data.json'
}
2022-10-19T23:27:47.506Z	173a20e7-ba14-4fa6-b23b-231c674bbf84	ERROR	null
RequestId: 173a20e7-ba14-4fa6-b23b-231c674bbf84 Error: Runtime exited with error: exit status 1
Runtime.ExitError

Expected Result

data.json is included in *.nft.json files for all tested @sentry/nextjs versions.

Actual Result

data.json is included in *.nft.json files for @sentry/nextjs@7.13.0 and @sentry/nextjs@7.14.2, but not for @sentry/nextjs@7.15.0 and @sentry/nextjs@7.16.0. When the app is deployed to Vercel without data.json in *.nft.json, lambdas crash because the cannot open the file.

Links

@lforst
Copy link
Member

lforst commented Oct 20, 2022

Hi, thanks for reporting this and providing a ton of information. I am able to reproduce this.

The autoInstrumentServerFunctions is 100% responsible for this. In the meanwhile, to get your stuff working again, you can set autoInstrumentServerFunctions: false in next.config.js (docs).

We did something somewhat risky with this feature and issues like these just simply didn't come up during development - sorry for the inconvenience! If you figure out any workarounds or have solutions in mind, let us know!

We'll try to find a fix.

@kachkaev
Copy link
Author

kachkaev commented Oct 20, 2022

Thanks @lforst! Setting autoInstrumentServerFunctions to false worked: blockprotocol/blockprotocol#694.

Happy to try out new ideas Sentry team comes up with 👍

@clarkexcore
Copy link

clarkexcore commented Oct 20, 2022

My team is also experiencing this same issue. Trying the autoInstrumentServerFunctions: false fix now. For us it was giving us this error: ERROR Error: ENOENT: no such file or directory, open linking to our email template files.

Edit: as per @kachkaev autoInstrumentServerFunctions: false fix is working and we are no longer receiving the error.

@lobsterkatie
Copy link
Member

lobsterkatie commented Oct 25, 2022

@kachkaev - Echoing @lforst here: Thanks for such a thorough writeup and investigation!

It's not entirely clear yet, but fixing this may mean trying to contribute an upstream patch to the nft package. I'm going to dive into it for a bit and see what I can figure out, then ping vercel folks in a few days once they're no longer actively in the middle of their nextjs conference.

UPDATE 1: Checked out the nft files you linked and ugh - the problem is actually worse than I realized. Yes, data.json is gone, which is a problem. But the map files added in the 7.13/7.14 versions ought not to be there (they're big and not helpful at runtime given that everything is running on an inaccessible AWS server somewhere), and a great number of the files added in the 7.15/7.16 versions also ought not to be there, since they're only needed at build time, not runtime. At the same time, I'm wondering why there are no sentry files at all in the 7.13/7.14 versions... 😓

UPDATE 2: Okay, question 1 solved. The 7.13/7.14 versions have no sentry files because the API route isn't using withSentry. Sentry stuff does show up in the _app nft file when I clone your repo and build locally.

Oy. Okay. Time to clone the nft repo! 🙂

@alechartung
Copy link

alechartung commented Nov 1, 2022

I think we're running into a similar issue here, except with a different package.
The only api routes we have in our next.js application are with https://github.com/auth0/nextjs-auth0, and disabling autoInstrumentServerFunctions has things working again.

@lobsterkatie
Copy link
Member

@alechartung - Sorry, I'm not sure what you mean. This issue is specifically about the nft files missing dependencies from the page files we wrap. How does nextjs-auth0 play into it?

@alechartung
Copy link

Maybe it's not the same exact issue and just has the same workaround, then.

@lobsterkatie
Copy link
Member

Yes, it turns out there are any number of ways for the autowrapping to not play nicely with other packages. Would you please file a separate issue and give the details of the error you’re running into?

@adamduncan
Copy link

Appreciate the thorough write-up, @kachkaev 👍 We're running into the same issue. Trying to readFile from our API routes to generate emails from template files on the server.

Have forked the MWE repo, upgraded @sentry/nextjs to the latest 7.21.1, but the issue still persists.

Our current workaround is to use its new excludeServerRoutes config option to ignore this particular API route that needs filesystem access. E.g.

const moduleExports = {
  sentry: {
    excludeServerRoutes: ['/api/hello']
  }
}

@lobsterkatie
Copy link
Member

lobsterkatie commented Dec 3, 2022

I have a PR open in the nft repo to fix the underlying cause of this problem. Until that's merged, if you use patch-package, here's a patch I made for my test app which applies the changes from the PR. (UPDATE: I realized that to make this work you also need dependencies from @vercel/nft, which I already happened to have. Probably easier to just wait for the fix to merge...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment