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

ref(nextjs): [Experiment] Automatically wrap API route handlers #3469

Closed
wants to merge 2 commits into from

Conversation

lobsterkatie
Copy link
Member

@lobsterkatie lobsterkatie commented Apr 28, 2021

This PR uses webpack to wrap API request handlers in a try/catch, so that errors can be reported to Sentry. Currently, that wrapping must be done manually, using withSentry. This makes that process automatic.

The current implementation is pretty hacky - definitely open to suggestions of a better way. Regardless, here's how it works:

  • We create a webpack loader, which then is added to the processors which run on API route files. It's passed a stringified version of the code from each file in question, and returns a stringified version of the code after it's been wrapped.
  • In order to do the wrapping, we need the code as code, so we feed it to Node's internal compilation method.*
  • We do the wrapping.
  • In the stringified original code, we replace the original handler with our wrapped one. Literally. Like with string replacement. 🤦🏻‍♀️
  • Since the wrapped function has a reference to the original function, we add the code of the original function back in, but as a function expression rather than an export.

*Unfortunately, in order to instantiate a Module object to use to do the compilation, we need the code we pass it to use commonJS-style imports, which there's no guarantee it does. So before we pass it to our loader, we run it through Babel, using a Babel plugin specifically designed for that exact transformation.

@HazAT HazAT changed the title ref(nextjs): Replace plugin ref(nextjs): Replace next.js plugin Apr 28, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Apr 28, 2021

size-limit report

Path Size
@sentry/browser - CDN Bundle (gzipped) 20.63 KB (+0.01% 🔺)
@sentry/browser - Webpack 21.5 KB (0%)
@sentry/react - Webpack 21.53 KB (0%)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped) 27.92 KB (+0.01% 🔺)

@HazAT HazAT changed the title ref(nextjs): Replace next.js plugin ref(nextjs): [Webpack Loader Experiment] Replace next.js plugin Apr 28, 2021
@HazAT
Copy link
Member

HazAT commented Apr 28, 2021

We will ship the manual approach today #3462
this PR is still too unstable and doesn't work in all of our manual test cases.
We'll try to make it work tho in the coming days.

@lobsterkatie lobsterkatie changed the title ref(nextjs): [Webpack Loader Experiment] Replace next.js plugin ref(nextjs): [Experiment] Automatically wrap API route handlers May 5, 2021
@lobsterkatie lobsterkatie marked this pull request as draft May 5, 2021 17:33
@ctrlaltdylan
Copy link

Woof heroic effort.

Makes me think this is a shortcoming with NextJS and not so much a Sentry problem.

If they had a dev friendly way of defining groups of API routes and wrapping handlers, then we wouldn't need this kind of workaround to inject wrappers.

@lobsterkatie
Copy link
Member Author

This is out of date and whatever we decide to do here, we should start from the current version of the SDK, using this for reference if it turns out to be useful. Closing.

@lobsterkatie lobsterkatie deleted the kmclb-nextjs-post-plugin branch September 15, 2021 05:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants