Skip to content
This repository has been archived by the owner on Jun 15, 2023. It is now read-only.

Update example with most recent NextJS and server-side sourcemaps #8

Merged
merged 4 commits into from
Jan 6, 2023

Conversation

BethanyBerkowitz
Copy link
Contributor

@BethanyBerkowitz BethanyBerkowitz commented Dec 28, 2022

In the process of working on https://github.com/honeybadger-io/docs/issues/190, I thought it would be useful to get this repo working with the most recent version of NextJS, including getting server-side sourcemaps working, which has been an issue in the past.

In the current state of this PR, I get all the example errors reported to Honeybadger, with sourcemaps applied, either when I'm running the app locally (ie npm run build && npm start) or when I deploy to Heroku. I have issues when deploying to Vercel (see below), but I believe those are longstanding issues and are not caused by this PR.


When I deploy to Vercel, I can't get server-side errors at all (on my branch or on master). All the server pages give me a 404. The Vercel log looks like this:
Screen Shot 2022-12-28 at 10 49 57 AM
So it appears that the function is getting called and erroring as I'd expect (500) but then displaying a 404. I don't yet understand why the "Edge Status" differs from the "Function Status".

I did confirm that there's nothing wrong with the routing, by adding another page within the /server folder that does not throw an error. That page loads fine on Vercel, so it's definitely related to error handling and not a true 404.

_error.js is getting called, however it's getting called for the 404, not the 500. The err object is null.
Screen Shot 2022-12-29 at 9 55 36 AM

Based on comments from @subzero10 I do not think these Vercel issues were introduced by this PR.

}).beforeNotify((notice) => {
notice.backtrace.forEach((line) => {
if (line.file) {
line.file = line.file.replace(`${ projectRoot }/.next/server`, `${process.env.HONEYBADGER_ASSETS_URL}/..`)
Copy link
Contributor Author

@BethanyBerkowitz BethanyBerkowitz Dec 28, 2022

Choose a reason for hiding this comment

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

Based on https://github.com/honeybadger-io/nextjs-with-honeybadger/compare/node-source-maps?expand=1#diff-c85e043f1db00b919581c615d87508f7d4ff2bb8058798ebbe475b4a6e087f26, but I had to add /.. to get sourcemaps working for me (running locally). I'm concerned that this might not be universal, however. And this whole thing feels a bit janky... I hope we do not need to ask users to copy/paste this whole thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This also seems to work when deployed to Heroku, so I am inclined to merge it in. I still think asking users to copy/paste all of this is not ideal, but it's better to have an ugly example that works while we think of something more elegant.

Copy link
Member

Choose a reason for hiding this comment

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

while we think of something more elegant.

I think the solution is to release a nextjs dedicated package that applies this specific configuration.

</ul>
</ul>
<h3>Server exceptions</h3>
<ol>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Minor, just wanted to number the list to make the testing easier visually.

<li>
getInitialProps throws an Error. This should cause _error.js to render
and record Error('Client Test 1') in Honeybadger.{' '}
<Link legacyBehavior href="/client/test1">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding legacyBehavior should make these links work in NextJS 13 as well as older versions.

@subzero10
Copy link
Member

@BethanyBerkowitz To answer your question:

Were the server-side errors ever being reported correctly when deployed to Vercel? Ie is this a new issue?

Personally, I was never able to get server errors reported on Honeybadger when running on Vercel.
I was able to get server errors only when running production mode (NODE_ENV=production) locally.
This led me to believe that something internally to Vercel is limiting us from reporting the errors. We faced a similar issue with our AWS Lambda integration, where we have to tweak the uncaughtException listener. The same or a similar workaround might be necessary for Vercel as well, but that's where I stopped my investigations.

@BethanyBerkowitz BethanyBerkowitz changed the title [WIP] Update example with most recent NextJS and server-side sourcemaps Update example with most recent NextJS and server-side sourcemaps Jan 3, 2023
@BethanyBerkowitz
Copy link
Contributor Author

BethanyBerkowitz commented Jan 3, 2023

@BethanyBerkowitz To answer your question:

Were the server-side errors ever being reported correctly when deployed to Vercel? Ie is this a new issue?

Personally, I was never able to get server errors reported on Honeybadger when running on Vercel. I was able to get server errors only when running production mode (NODE_ENV=production) locally. This led me to believe that something internally to Vercel is limiting us from reporting the errors. We faced a similar issue with our AWS Lambda integration, where we have to tweak the uncaughtException listener. The same or a similar workaround might be necessary for Vercel as well, but that's where I stopped my investigations.

Thanks for this info, @subzero10! In that case, I'm inclined to merge this PR even though I haven't addressed the Vercel problems and create a new ticket to look further into it.

Copy link
Member

@joshuap joshuap left a comment

Choose a reason for hiding this comment

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

This looks good to me.

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

Successfully merging this pull request may close these issues.

3 participants