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

NW2: Crashing renderer shows Aw, Snap! #7339

Open
gpetrov opened this issue Jan 24, 2020 · 7 comments
Open

NW2: Crashing renderer shows Aw, Snap! #7339

gpetrov opened this issue Jan 24, 2020 · 7 comments
Labels

Comments

@gpetrov
Copy link

gpetrov commented Jan 24, 2020

NWJS Version : 0.43.6
Operating System : Windows 10 and MacOS

Expected behavior

When the renderer process crashes, the NWJS App should end.

Actual behavior

Now when the render process crashes an d "Aw, Snap!" page is shown with option to retry - but retry doesn't work.

aw_snap_renderer_crash

after retry:
after_reload_renderer_crash

Also if an endless loop or heavy duty JS tasks are busy, after 25 secs a dialog shows that the page is busy:

pageUnresponsive_loop

Also on Mac in the console errors appear:
[0124/170204.391511:WARNING:system_snapshot_mac.cc(42)] sysctlbyname kern.nx: No such file or directory (2)

Same behavior is shown on Windows as well.

How to reproduce

Use the supplied test case:

test_crash.zip

When NW2 is disabled then - the rendere crash just closes NWJS.

Ideal crash reporting

@rogerwang We love the NW2 and it's great that it has the Chrome build in error reporting, we just need a better way to handle them. Ideally we would like to popup a window for error reporting allowing the user to submit a error report for the crash.

Maybe something like:

https://www.electronjs.org/docs/api/crash-reporter

and also integration with Sentry that we already use for our NWJS error reporting:
https://docs.sentry.io/platforms/javascript/electron/

Currently we get the minidump reports, but only on browser crash and not renderer. And there is no way for the user to enter additional info. NWJS just dies which is a very bad UX.
Also besides the minidump - we can submit additional state data, like collected JS stack trace, which will be really useful.

@gpetrov
Copy link
Author

gpetrov commented Jan 24, 2020

Also as addition if a frameless window is used, then the Aw Snap window can never be closed!

@rogerwang
Copy link
Member

Before providing new API for error page customization, I'll disable the 'reload' button, probably replacing it with 'close'.

@gpetrov
Copy link
Author

gpetrov commented Jan 25, 2020

actually I think the API already exists in the form of chrome.processes

From there we can monitor if a tab "exited" - died (exit code != 0)
https://developer.chrome.com/extensions/processes#event-onExited

or "hung":
https://developer.chrome.com/extensions/processes#event-onUnresponsive

The question is only where to add this monitoring hook? As on crash/hung seems both the background and the browser devtools/scripts die. At least both devtools gets immediately disconnected...

Maybe in the separate bg-script background page entered at the manifest?

@rogerwang
Copy link
Member

Thanks for the link. Those APIs are supposed to be available in NW. But NW uses a different process model -- web pages lives in the same process unless the window is created with new_instance -- so when a process dies, the background page dies with it.

@rogerwang
Copy link
Member

The nightly build for this issue is available at https://dl.nwjs.io/live-build/nw44/20200125-162000/5925084b5

@bodo22
Copy link

bodo22 commented Feb 19, 2020

Before providing new API for error page customization, I'll disable the 'reload' button, probably replacing it with 'close'.

With nw1, when testing a renderer crash with nw.App.crashRenderer(), the whole nw process dies. We rely on this fact in a kiosk setting to auto-restart nw. With nw2, the nw process is not killed completely. As there is no API for handling renderer crashes in nw2 yet, is there a workaround for non-gracefully killing nw in the interim? Or would it make sense to handle crashing differently based on a manifest file parameter, e.g. if kiosk is set, or to make it more explicit, a new parameter?

@rogerwang
Copy link
Member

@bodo22 please file another issue. I'll make the behavior the same as nw1. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants