-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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] @sentry/cli always bundled in production #3865
Comments
webpack plugin has been moved to dev deps. Let me know if this issue will still persist after next release. |
Hmmm. Not having seen this, I moved it back, because SDK users need it as part of building their app, and if it's in our dev dependencies, it'll only install for us, not for folks who yarn install our SDK.
UPDATE: Stumbled across an undocumented I notice that we're also both ending up with In any case, as it happens, I'm actually trying to get more insight into how the lamdas are created for other reasons (was working on it just today, in fact), so perhaps getting a handle on that will shed some light here as well. |
Reopening this because it isn't solved now that I undid the change. |
@lobsterkatie sorry for long delays, I haven't looked thoroughly, but yes moving to devDeps is quite a strong statement somehow :) Something that might help you: Working with NEXT_DEBUG_FUNCTION_SIZE is not very easy when going testing. Personally I created a little wrapper over https://github.com/vercel/nft to trace what is included in the server build. Very convenient. I haven't had the time to look into the code, but I'll try next week. |
Spend the past couple days to debug this exact issue :/ I'm working on a NextJS app, deployed on Vercel. We're using the @sentry/nextjs integration and have one serverless api function that makes screenshots using But because of this issue sentry is adding a couple megabytes to the compressed lambda function, which brings it over the max limit of 50MB and therefore fails the build. Even worse, the sentry plugin seems to include a lot more unrelated dependencies in the lambda function. This issue a is total roadblock for us: Is there any workaround or do I need to stop using sentry? |
If you're hitting serverless limits the @sentry/nextjs is not the way to go IMHO. (I would add if you're going close to 40Mb, lambdas would already start giving 504 from time to time, cold starts are too long depending where you host them). As workaround it's possible to use sentry for the browser part only, you'll have to go back in time in the examples, ie https://github.com/vercel/next.js/tree/6aef4b8445472c53c4a4c39005319c2fc73e44de/examples/with-sentry. that's more or less the direction I took for sandboxes (prod is on docker/k8s so it's less a problem). For monitoring api routes we actually send to datadog. But I feel there must be some more thinking related to @sentry/nextjs. Personally and if it's only used to upload sourcemaps I would ditch @sentry/cli totally in favour of lightweigtht js code (or even wasm). I fear that It would be almost impossible to shrink the size of the binaries in the current state (not even talking that on alpine the wrong binary is picked when installing and the fact that sentry.properties looks weird for js devs, etc). Nextjs, lambdas, serverless... paradigms requires deps to be light as possible. @sentry/nextjs should consider this as a requirement to ease adoption. Cause both sentry and nextjs are wonderful products 😄 PS: @stnwk your use case with pupeeter might look very specific... But actually in many setup we'll add sharp, prisma, graphql and why not redis ?.. add @sentry/nextjs and the limit is easily reached. |
Yeah, we have all of those too 😅 It's definitely an issue that many people will run into. Thanks for your quick response! I'm currently trying to find a way without So far, so good. Only nextjs api routes remain a problem, let's see. |
There's two builds, one for server code, one for client code. I guess standard webpack and tree-shaking is not used to optimize lambdas "as is". (anyway if a package require @sentry/cli and in return @sentry/cli loads a binary it would be hard for the tree-shaking to happen. There might be a clue here.) I'm not sure how nextjs does, but I assume they use @vercel/nft package to do a tracing of files (not packages, but files present in node_modules) that are required from the route (lambda, ie: /api/test). From there they will probably try to remove all (node_modules) files that aren't actually needed.(node_modules is a black hole, so without this lambdas wouldn't possible), then keep the required ones and do a specific bundle with them. I assume the reason why the sentry binary is included is that there's a require of it (cause process.env.NODE_ENV is 'production' when you're in the build phase) There might be something to do here. I don't know how vercel does, but you can have a look of how serverless does here: https://github.com/serverless-nextjs/serverless-next.js/blob/master/packages/libs/lambda-at-edge/src/build.ts.... It's possible that other providers use this method (netlify...). I haven't investigated cause I'm using docker/k8s for prod. So technically it would be possible to ditch the binary by adding exclusion in @vercel/nft and then coordinating with nextjs, serveless, aws-amplify.... But IMHO, if it's just to upload the sourcemaps I would create a lightweight js script, for those reasons:
It's just some comments I have in my mind. but no real solution, hope you'll find one 💯 |
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 "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
Leave it. |
Ah I'm following vercel/next.js#30401, it seems there's some move, sentry at its best 😄 Thx a lot @lobsterkatie , if anytime I can help to test ping me. |
We are having this same issue (Sentry + Puppeteer + ChromeAWSLambda) with 2 projects and so far the only solution is to use the classic webpack-alias solution 😢 Let us know if there is other possibility to take advantage of this official solution |
Same issue here! Any updated? And @stnwk, could you clarify on
and maybe provide an example? |
Sorry, I no longer work in this context and don't remember the exact details from August last year. What I meant with this:
is, that Back then I was trying to implement a work-around using webpack aliases and the webpack-plugin directly instead of using @sentry/nextjs. But that's about all I remember here. |
Hi folks, I know this is an outstanding issue, and apologies we have not been able to give it more attention. We are very focused on the work #4882 detailed here and the roadmap linked therein. There will be significant bundle size reduction with the V7 release. Rest assured we are very bundle size sensitive these days 😬 We will revisit this as capacity sees fit! |
we are currently investigating 3 possibilities
The 3rd option being what we agreed on with Vercel, as a fix that can be done prior to/instead of making some tree shaking changes. This does add some complexities, and similar to electron it might have some problem for some users with certain setup. But we are dedicated to finding a solution that will work for most everyone. |
build is failing....
|
Hi all! We've recently shipped our first beta for the V7 release! We would love to get your feedback on the changes we've made to the SDK and migration guide. Also important to note the fix for @sentry/cli always bundled in production is one of the improvements. Really hoping to hear back about some bundle size wins :D Thank you! |
Very disappointing... Still same error with
My nextjs project does not even run locally on development mode. |
@stavalfi hmm yes disappointing :( this is not the same as issue as how this issue started. You are blocked entirely vs just having all the bloat from CLI in prod bundle |
@stavalfi you need to provide project name for your cli config, as the error states - https://docs.sentry.io/platforms/javascript/guides/nextjs/manual-setup/#use-configuration-files |
Here's some quick tests with On the server sidesentry-cli is well removed 💯, congrats ! Tested on vercel (lambdas) with latest nextjs 12.1.6 Unrelated to this issue, but worth to share: ThrottlingWhen uploading sourcemaps the requests are often throttled On the browser side7.0.0-beta.1 made my bundle size happier (with tracing disabled). Amazing Modest P/R example here: belgattitude/nextjs-monorepo-example#1854 Keep up the fantastic work ! |
@belgattitude - https://docs.sentry.io/platforms/javascript/guides/nextjs/configuration/tree-shaking/ should give you even more savings! Thanks for the feedback, this is something we'll keep improving on! |
I guess Attempt here belgattitude/nextjs-monorepo-example#1863, but haven't seen any significant difference (58kb vs 59kb for sentry ). Tested with Always open to ideas, I'll keep giving some feedbacks along the beta's, let me know if you want to try out things. Keep up shining. |
closing this issue now that the fix is shipped and folks are reporting success 😄 for issues around how to better take advantage of the bundle size reductions in V7, including tree shaking, we might be better off moving those talks here #4240 to include more folks
many thanks for your support and feedback @belgattitude! :) it's been a long road to V7, happy folks appreciate the work |
Package + Version
@sentry/nextjs
- v 6.10.0@sentry/webpack-plugin
- v 1.16.0Version:
Description
Right now the
@sentry/webpack-plugin
(/@sentry/cli
) is required as regular dep, meaning we can't get rid off it in production.This makes prod bundle much bigger and lead to
As an example, see a vercel debug based on an example repo https://github.com/belgattitude/nextjs-monorepo-example
Edit: Try a deploy on vercel and check the log with NEXT_DEBUG_FUNCTION_SIZE=1
Fixing
It's not totally clear to me what would be the best to do.
Option 1:
@sentry/nextjs
could move@sentry/html-plugin
to devDependencies (or peerDeps/optional), but I'm not sure as it's included in nextjs.config.js so maybe need some more work to be sure the plugin is not required for production builds. A proposal here: #3866Option 2:
@sentry/html-webpack
plugin, move@sentry/cli
to devDependencie (or peerDeps/optional)... That would be sufficient as the it's actually the one that increase size significantly (17Mb).Edit
Option 3:
Ditch @sentry/cli in favour of a custom lightweight sourcemap uploader (That would be the more universal solution)
Option 4:
Possible too, add an option to vercel/nft / nextjs to ignore sentry binary... Would solve vercel problem, but then quid of serverless, netlify...
I could provide some P/R's but let me know what would the recommended approach...
The text was updated successfully, but these errors were encountered: