Handle AbortErrors when calling commands (eg "browsingContext._getBaseURL")
Categories
(Remote Protocol :: WebDriver BiDi, defect, P1)
Tracking
(firefox103 fixed)
Tracking | Status | |
---|---|---|
firefox103 | --- | fixed |
People
(Reporter: jdescottes, Assigned: jdescottes)
References
(Blocks 2 open bugs)
Details
(Whiteboard: [webdriver:m4])
Attachments
(3 files)
While investigating Bug 1763692, I intermittently had issues with handleCommand throwing with an AbortError, because the JSWindowActor pair we relied on was destroyed while waiting for the query.
This is something we also faced with Marionette and handled by adding a proxy to "retry" queries in Bug 1671347
For the initial MessageHandler implementation, I did not want to have a retry mechanism built in by default, because I felt like it could hide issues.
But we should start discussing how we want to handle that. Do we want a low-level (query) retry mechanism, same as in marionette. Do we want to handle it at command level?
Assignee | ||
Comment 1•1 year ago
|
||
We could add a default behavior (retry or not) and then allow each call to a command to override this.
Assignee | ||
Comment 2•1 year ago
|
||
Since Bug 1767226 started spiking, let's try to fix this.
Updated•1 year ago
|
Assignee | ||
Comment 3•1 year ago
|
||
Only commands sent to a windowglobal MessageHandler are impacted here.
And for now, this is only about browsingContext._getBaseUrl
.
All other commands are either fully handled in root
modules, or are about events which get propagated using SessionData and therefore are not subject to AbortError.
However the very next command we plan to implement, script.evaluate
, will be sent explicitly to a windowglobal MessageHandler, so it's probably worth at least having this use case in mind when fixing this bug.
Assignee | ||
Comment 4•1 year ago
|
||
Two private methods which were still listed as internal commands.
Assignee | ||
Comment 5•1 year ago
|
||
Depends on D147713
This adds a "retryOnAbort" property on MessageHandler Commands.
When set to true, this will allow the FrameTransport to retry a command up to 10 times in case of an AbortError.
Difference with Marionette, this doesn't attempt to detect BrowsingContexts in bfcache, because retrying commands
does not make sense in bfcache (this is not a temporary state, retrying in a loop will most likely not help).
A browser mochitest is added to cover various retry scenarios.
Assignee | ||
Comment 6•1 year ago
|
||
Depends on D147715
No test added, hopefully this should address intermittents such as Bug 1767226
_getBaseURL is so far the only command explicitly sent to a windowglobal MessageHandler,
and therefore is the only one we should consider for the retry behavior.
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ad52948eb92f [bidi] Cleanup private methods for events in windowglobal/log module r=webdriver-reviewers,whimboo https://hg.mozilla.org/integration/autoland/rev/ac0842a7d090 [remote] Allow MessageHandler commands to be retried upon AbortError r=webdriver-reviewers,whimboo https://hg.mozilla.org/integration/autoland/rev/72e63add1c92 [bidi] Allow browsingContext.navigate to retry the _getBaseURL command r=webdriver-reviewers,whimboo
Comment 8•1 year ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ad52948eb92f
https://hg.mozilla.org/mozilla-central/rev/ac0842a7d090
https://hg.mozilla.org/mozilla-central/rev/72e63add1c92
Description
•