-
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(driver): add sendCommandAndIgnore #15913
Conversation
.catch(_ => {}); | ||
void session.sendCommand('Runtime.terminateExecution').catch(_ => {}); | ||
void session.sendCommandAndIgnore('Emulation.setScriptExecutionDisabled', {value: true}); | ||
void session.sendCommandAndIgnore('Runtime.terminateExecution'); |
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.
Thoughts on making this return void, doing the void *
hack inside the method?
if the semantics is just "toss this over to cdp and move on" we don't need to await anything.
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.
hmmm yeah. i'm up for tweaking it.
the void *
thing (generally a convention from the devtools codebase) is for saying.. i'm not awaiting or returning this promise. https://typescript-eslint.io/rules/no-floating-promises/#ignorevoid
but the new AndIgnore method is for.. catching the rejection so it doesn't continue (or go unhandled)
they're slightly different things, right?
and it seems like we have cases where we want to await the AndIgnore calls .. and cases where we don't want to await them
So i think this new method should still return a promise. but.. it can be empty
and given this tweak, maybe ignore
isn't a great fit. wdyt sendCommandQuietly
? open to other names. (but also no big whoop on naming)
54c45d4
to
6cc86bd
Compare
followup from #15833 (review)