1790375 - Implement "network.fetchError" event

Status: RESOLVED FIXED

(Remote Protocol :: WebDriver BiDi, task, P2)

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Adding as tentative bug for our M6 milestone.

Comment 2

[Julian Descottes [:jdescottes]]

Note that the errorText field of the event payload is not in the specs yet:

TODO: Set the errorText field of params.

Comment 3

[Julian Descottes [:jdescottes]]

I filed https://github.com/w3c/webdriver-bidi/issues/631 about the missing errorText definition.

Comment 4

[Julian Descottes [:jdescottes]]

Hi Valentin,

I'm trying to use the "stop" and "error" events on the ChannelWrapper in order to know if a request succeeded or failed. In most situations this works fine, but I have an issue on Android where redirects emit both "error" and then "stop". On Firefox Desktop, only "stop" is emitted in the same scenarios.

This seems to happen for any redirect, but for instance this shows up in the following test: https://searchfox.org/mozilla-central/rev/24ea2579c4a94d5da8db4bb529cbefe5e5e3c2d3/testing/web-platform/tests/webdriver/tests/bidi/network/response_completed/response_completed.py#308-370

I can work around that by ignoring channels which are failing with the error "NS_BINDING_REDIRECTED", but that leads to another problem. If I am redirecting to a URL that should result in a fetch error, on Android the error will still be "NS_BINDING_REDIRECTED", whereas on desktop I will have the correct error (eg NS_ERROR_UNKNOWN_HOST).

Do you know if there are issues/limitations with redirected requests on Android, or should I file a dedicated bug for that?

Comment 5

[Julian Descottes [:jdescottes]]

Also pinging Olivia for the question above, since I only tested with the testrunner on Android. Could it be something that also needs to be fixed in the android testrunner?

Comment 6

[Julian Descottes [:jdescottes]]

Attachment: Bug 1790375 - [bidi] Implement network.fetchError event

Implements https://w3c.github.io/webdriver-bidi/#event-network-fetchError

Comment 7

[Julian Descottes [:jdescottes]]

Attachment: Bug 1790375 - [bidi] Listen to _fetchError when waiting for navigation

Depends on D197586

Comment 8

[Julian Descottes [:jdescottes]]

Attachment: Bug 1790375 - [wdspec] Add tests for bidi network.fetchError event

Depends on D197587

Comment 9

[Julian Descottes [:jdescottes]]

Attachment: Bug 1790375 - [wdspec] Increase timeout in network.fetchError tests for windows

Depends on D197588

On Windows CI, the fetchError takes significantly longer to be emitted than on other platforms so I had to increase all the timeouts.
Another option would be to mark those as fail and investigate why Windows needs more time to error channels

Comment 10

[Julian Descottes [:jdescottes]]

Added as a block for network.failRequest because the fetchError event is needed in order to write proper tests for failRequest

Comment 11

[Julian Descottes [:jdescottes]]

Clearing the needinfos for now, as the current stack no longer has the issue I mentioned. Will push to try with an older version of the patch to help with the investigation.

Comment 12

[Julian Descottes [:jdescottes]]

Here's a try push you can use to reproduce the issue: https://treeherder.mozilla.org/jobs?repo=try&revision=7d14de289b76f7d2d3e859bb7fda8145f9c24f15

You can pull the patches, build for Android and run the test testing/web-platform/tests/webdriver/tests/bidi/network/response_completed/response_completed.py. There are some additional logs as well, but for android, the redirected request will lead to 2 "error" events emitted on the ChannelWrapper. Instead on desktop, we get 2 "stop" events for the same test.

I need to clarify something about my initial comment: when I said

on Android the error will still be "NS_BINDING_REDIRECTED"

I was referring to ChannelWrapper.errorString. When retrieving the error name from the channel's status, we get NS_BINDING_REDIRECTED and NS_OK, as expected. So this whole thing might be a bug with ChannelWrapper.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aaee4331946c
[bidi] Implement network.fetchError event r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/a51daae37008
[bidi] Listen to _fetchError when waiting for navigation r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/12e0d45fb593
[wdspec] Add tests for bidi network.fetchError event r=webdriver-reviewers,whimboo

Comment 14

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/43942 for changes under testing/web-platform/tests

Comment 15

[Cosmin Sabou [:CosminS]]

Backed out for causing windows wd failures on fetch_error.py.

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=51d3d025e7fa3113193da45b9c1b1d792060a4c3&searchStr=wd&group_state=expanded&selectedTaskRun=a61AtTUBRC-ZgzZScl_r_A.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=443003765&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/f5677037347b812cbf58927a01a6d1b1fb5f721a

Comment 16

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 17

[Julian Descottes [:jdescottes]]

Sorry about that, I did a typo in the in the test ini file Patch instead of PATCH.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a4e026a249cc
[bidi] Implement network.fetchError event r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/8d2f317d47e6
[bidi] Listen to _fetchError when waiting for navigation r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/cf5aaf41bb8e
[wdspec] Add tests for bidi network.fetchError event r=webdriver-reviewers,whimboo

Comment 19

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/a4e026a249cc
https://hg.mozilla.org/mozilla-central/rev/8d2f317d47e6
https://hg.mozilla.org/mozilla-central/rev/cf5aaf41bb8e

Comment 20

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Julian, should we maybe move the remaining issue from comment 12 to it's own bug?