-
-
Notifications
You must be signed in to change notification settings - Fork 6.1k
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: avoid early error when server is closed in ssr #13787
fix: avoid early error when server is closed in ssr #13787
Conversation
Co-authored-by: Bjorn Lu <34116392+bluwy@users.noreply.github.com>
Run & review this pull request in StackBlitz Codeflow. |
/ecosystem-ci run |
📝 Ran ecosystem CI: Open
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems easier than expected!
@@ -57,6 +57,7 @@ export async function createServer( | |||
res.status(200).set({ 'Content-Type': 'text/html' }).end(html) | |||
} catch (e) { | |||
vite && vite.ssrFixStacktrace(e) | |||
if (isTest) throw e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My test could be flaky as I haven't figured out some case where there's actually unexpected but harmless errors. I'm ok with leaving this out in case it makes our CI unstable 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI looks good for now. If we see it starts to bring issues we can remove it later.
Fixes #13735
Fixes #13786
Closes #13767
Description
Throwing an error during SSR would force the users to handle it on every call to
ssrLoadModule
. What is safer long term is something we should discuss for Vite 5. At least for 4.4 (and I'm leaning towards this approach for 5 too), it is better to avoid the error during SSR. The only reason it was added was to speed up server restart. During SSR, the requests should be fast enough.What is the purpose of this pull request?