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

fix(ssr): preserve fetchModule error details #18626

Merged
merged 1 commit into from
Nov 11, 2024

Conversation

hi-ogawa
Copy link
Collaborator

@hi-ogawa hi-ogawa commented Nov 9, 2024

Description

While investigating the other fix #18621, I realized new transport (introduced in #18362) is not preserving some error properties such as RollupError.loc, frame, which is useful for error overlay.

Error overlay screenshots

image

image

@@ -213,6 +213,7 @@ export const normalizeHotChannel = (
name: error.name,
message: error.message,
stack: error.stack,
...error, // preserve enumerable properties such as RollupError.loc, frame, plugin
Copy link
Collaborator Author

@hi-ogawa hi-ogawa Nov 9, 2024

Choose a reason for hiding this comment

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

Alternatively we can return Error instance directly { e: error } and let each transport implementation deal with it. This works for default ssr runner, but for others using JSON.stringify, it'll end up empty since JSON.stringify(new Error("hmm")) === '{}', so it's certainly a minor inconvenience for them.

Copy link

pkg-pr-new bot commented Nov 9, 2024

Open in Stackblitz

pnpm add https://pkg.pr.new/vite@18626

commit: dd1a756

@hi-ogawa hi-ogawa marked this pull request as ready for review November 9, 2024 03:01
@sapphi-red sapphi-red added feat: ssr p4-important Violate documented behavior or significantly improves performance (priority) labels Nov 11, 2024
@patak-dev patak-dev merged commit 866a433 into vitejs:main Nov 11, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: ssr p4-important Violate documented behavior or significantly improves performance (priority) trigger: preview
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants