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

renderToString() swallows errors #7876

Closed
brillout opened this issue Mar 10, 2023 · 9 comments
Closed

renderToString() swallows errors #7876

brillout opened this issue Mar 10, 2023 · 9 comments
Labels
🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. has workaround A workaround has been found to avoid the problem scope: ssr

Comments

@brillout
Copy link

Vue version

3.2.47

Link to minimal reproduction

https://github.com/brillout/vue-renderToString-bug

Steps to reproduce

See https://github.com/brillout/vue-renderToString-bug#readme.

What is expected?

renderToString() swallows errors occuring in Vue SFC <script>.

Note that renderToString() does log the error (console.error(error)) but it doesn't throw it (throw error).

What is actually happening?

renderToString() should throw the error so that erroneous state can be detected and handled accordingly.

System Info

No response

Any additional comments?

I don't see any workaround.

Checking whether conole.error() was called during renderToString() cannot be used to detect whether an error occured, because renderToString() is async.

See vikejs/vike#687 – I consider this a critical bug as showing a blank page instead of an error page is a considerable UX degradation.

@baiwusanyu-c
Copy link
Member

wow !The logError method does not throw an exception error in the production environment, but only prints the error. I think this is done to avoid the exception error throwing and causing the program to stop or crash. Perhaps vue can add an option for the user to determine whether to throw an exception error in the production environment, but it is actually a big change, involving error handling at runtime, and also needs to expand the compiler options (for example, the slot call code is generated by the compiler, not simply modifying the runtime can be achieved)

@brillout
Copy link
Author

I consider this a critical bug as showing a blank page instead of an error page is a considerable UX degradation.

Just to reiterate that, from the perspective of vite-plugin-ssr users, I think this should labeled as ❗ p4-important bug.

@haoqunjiang
Copy link
Member

haoqunjiang commented Apr 18, 2023

Does app.config.errorHandler work for your use case?

I'm not familiar with the internals of vite-plugin-ssr, but when I added the following lines to the createApp function in renderer/app.js, it successfully showed a 500 page for me:

app.config.errorHandler = (err) => {
  pageContext.errorWhileRendering = err
}

@baiwusanyu-c
Copy link
Member

Are you successful in a production environment? I was troubleshooting and found that errorHandler seems to end up in the logError function, and logError just prints the error and never throws it in production (maybe I'm wrong 🤣)
@sodatea

packages/runtime-core/src/errorHandling.ts -- 》logError

@brillout
Copy link
Author

brillout commented Apr 18, 2023

Indeed, this seems to be a viable workaround.

@baiwusanyu-c This is how I made it work: brillout/vue-renderToString-bug@68c0707.

@haoqunjiang haoqunjiang added the has workaround A workaround has been found to avoid the problem label Apr 19, 2023
@haoqunjiang
Copy link
Member

Now that there's a workaround, I would like to label this issue as "p3: minor bug".
This doesn't mean the issue isn't important, but we need first to fix the regressions, the serious bugs violating documented behavior, and those can't be worked around before coming to this one.

A side note:
There are a few other issues related to error handling. May be worth considering when deciding what to do so that we have better design and implementation consistency:

@haoqunjiang haoqunjiang added the 🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. label Apr 19, 2023
@brillout
Copy link
Author

Makes sense 👍 and thanks for the pointers.

@zhangyuang
Copy link
Contributor

I'm having the same problem. I hope there can be an option here to decide whether to throw an error or not.

@yyx990803
Copy link
Member

yyx990803 commented Jul 17, 2024

closed via f476b7f

This will be possible in 3.5:

app.config.throwUnhandledErrorInProduction = true

The default behavior cannot be changed because it would result in crashes in production for those who didn't previously set app.config.errorHandler.

@github-actions github-actions bot locked and limited conversation to collaborators Aug 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. has workaround A workaround has been found to avoid the problem scope: ssr
Projects
None yet
Development

No branches or pull requests

5 participants