Closed Bug 1790375 Opened 2 years ago Closed 3 months ago

Implement "network.fetchError" event

Categories

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

task
Points:
5

Tracking

(firefox123 fixed)

RESOLVED FIXED
123 Branch
Tracking Status
firefox123 --- fixed

People

(Reporter: whimboo, Assigned: jdescottes, NeedInfo)

References

(Blocks 3 open bugs, )

Details

(Whiteboard: [webdriver:m10][wptsync upstream][webdriver:relnote])

Attachments

(3 files, 1 obsolete file)

No description provided.
Blocks: 1790362
Priority: P1 → P2
Whiteboard: [webdriver:m5] → [webdriver:backlog]

Adding as tentative bug for our M6 milestone.

Priority: P2 → P1
Whiteboard: [webdriver:backlog] → [webdriver:m6]
No longer blocks: 1790362
Priority: P1 → P2
Whiteboard: [webdriver:m6] → [webdriver:backlog]
Priority: P2 → P1
Whiteboard: [webdriver:backlog] → [webdriver:m7]
Priority: P1 → P2
Whiteboard: [webdriver:m7] → [webdriver:m8]
Whiteboard: [webdriver:m8] → [webdriver:backlog]

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

TODO: Set the errorText field of params.

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

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?

Flags: needinfo?(moz.valentin)

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?

Flags: needinfo?(ohall)
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED

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

Blocks: 1873037
Whiteboard: [webdriver:backlog] → [webdriver:m10]
Blocks: 1853883

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

Blocks: 1873880
Attachment #9370890 - Attachment is obsolete: true
Blocks: 1874098

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.

Flags: needinfo?(valentin.gosu)
Flags: needinfo?(ohall)
Flags: needinfo?(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.

Flags: needinfo?(jdescottes) → needinfo?(valentin.gosu)
Pushed by jdescottes@mozilla.com:
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
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/43942 for changes under testing/web-platform/tests
Whiteboard: [webdriver:m10] → [webdriver:m10], [wptsync upstream]
Upstream PR was closed without merging

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

Flags: needinfo?(jdescottes)
Pushed by jdescottes@mozilla.com:
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
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 123 Branch
Upstream PR merged by moz-wptsync-bot

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

Flags: needinfo?(jdescottes)
Whiteboard: [webdriver:m10], [wptsync upstream] → [webdriver:m10][wptsync upstream][webdriver:relnote]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: