-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
fix(dev): increase default session memory limit in HTTPS dev server to prevent disconnections on large projects. #6207
fix(dev): increase default session memory limit in HTTPS dev server to prevent disconnections on large projects. #6207
Conversation
Thanks for creating this repro. For reference, @davidwallacejackson showcased the issue and this is terrible when it happens. I think we aren't getting more issues related to this problem because of the big amount of modules that are being loaded. Given that this option can already be configured, maybe we should document this problem and its solution instead of bumping the limit? If it was 20MB instead of 10MB, it wouldn't be a problem, but 1000MB is starting to sound that another issue may be hiding here and as you said... I would also not feel comfortable bumping it that high by default. |
I'm not sure how high it really needs to be -- 1000MB was what I did in my own project; it seems like 100MB fixes it as well but I wanted to leave a lot of room because my team had hit the problem a lot and I wanted to leave a LOT of room, knowing I could change it in another PR if I really had to. Haven't tried something lower, like 20MB, but for all I know that might solve it. Documenting it and letting users configure it also seems like a good idea -- I was actually thinking that a nice way to do it would be to listen for the 502 error specifically with https://nodejs.org/api/http2.html#event-error and emit an error message directing users to that setting, but I haven't tried wiring that up yet. |
Yeah, it seems another user had a similar issue before and had good luck with just setting it to 100mb and created a PR allowing users to specify |
This is awesome: #3895, no why it's closed. |
What's the status? |
We're also running into this issue, and pretty frequently. Seems to also happen not only when many modules are loaded, but simply when large files are served along the page (e.g. a 10-20MB GLB file or so). |
@davidwallacejackson, would you test again that the PR is working well for you? We talked about it today, we can merge it during the v3 beta and see if someone reports issues. |
Let's merge it so we can start checking how it behaves for others. |
Description
In projects with a particularly large number of modules, some requests to the dev server will fail when the server is under heavy load. Which requests fail is nondeterministic, but some of them always will. It looks like this:
Here's a repro. Unfortunately, I couldn't do this in StackBlitz -- when I tried to run it there, I couldn't even get the app to start. I'm assuming that's because this involves intentionally thrashing the filesystem and StackBlitz isn't optimized for that kind of fs access, but I don't really know.
Additional context
When I had this problem with my own app, I did a bunch of research on the
502 ENHANCE_YOUR_CALM
error that I was seeing in my terminal output. I eventually tracked it to nghttp2, the library Node uses to implement HTTP/2 support. This now-closed issue in Node itself gave me a clue as to what was going on. Apparently the error message is an anti-DDOS mechanism -- in the case of the now-fixed bug, the memory used by requests wasn't getting released.The default value for this limit is 10MB. I guessed that it might just be too small for a large-ish Vite project, so after seeing how the options get passed to
createSecureServer
, I bumped it up in my own project by passinghttps: { maxSessionMemory: 1000 }
in myvite.config.ts
, and this seemed to solve the problem consistently.100
also seemed to work, but it doesn't for this repro.Admittedly, the repro is absolutely bonkers. My real project loads ~1800 modules (we're working on code-splitting, but we did some things that inadvertently made that difficult, so it'll take a while 😬 ) but pulls down nowhere near as much data as the repro does. I'm not sure what a reasonable limit is, or what the dangers of bumping the default value might be (will it affect SSR?), so I'm curious what you all think about that. It also seems like something that should probably be configurable, but I had a bit of trouble trying to add it to the allowed types -- the option in question comes from
SessionOptions
, which doesn't make it into the set of allowed options as considered by Vite for reasons I haven't figured out yet.Also, if you'd like me to make my repro into a test, let me know! I wasn't sure if this sort of heavyweight thing was appropriate for the test suite, but happy to add it if it is.
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).