-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
core: handle target crash at any point #15985
Conversation
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.
bringing fatal promise over to the session is awesome. works great. let's go with that
for my own posterity.. i test this with adding this to gatherers/trace.js right after the Tracing.end
line:
setTimeout(() => {
session.sendCommand('Page.navigate', {url: 'chrome://crash'});
}, 100);
OK I realized the Driver shouldn't be the thing handling this crash promise. I just moved it to ProtocolSession, dropped all the wiring being used to get it there, and added a |
Oh, this new approach also means we handle crashes in all sessions, not just the root. |
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.
not gonna lie this is like.. sooooooooooooooo good.
net negative line count change.. and expanded the functionality. and its easier to follow.
just a huge win all around.
tested it a few ways and it looks good. i get an LHR regardless, whenever the crash happens.
NIIIIIIIIIIIIIIIIIIICE
#11840 added a specific error code when the chrome target crashes, but it only applied to navigations within the waitFor condition check. Any other runner, or during teardown in navigation, and a crash would still result in a PROTOCOL_TIMEOUT.
This PR adds the crash promise to ProtocolSession, and checks during any sendCommand that there was not a crash.
Alternatively, we can introduce a wrapper around gatherFn and check if the Driver is in a crash state. This PR has both approaches (thus Draft).