1484941 - Change timeout and script timeout error HTTP codes from 408 to 500

Status: RESOLVED FIXED

(Testing :: geckodriver, enhancement, P1)

Description

[Andreas Tolfsen ❲:ato❳]

Issue: https://github.com/w3c/webdriver/issues/1287
Spec change: https://github.com/w3c/webdriver/pull/1292

This bug will track changes to WPT tests, the webdriver crate, and
geckodriver.  The spec change cannot land until we have tests ready.

Comment 1

[Andreas Tolfsen ❲:ato❳]

Attachment: Change ErrorStatus::{ScriptTimeout,Timeout} HTTP codes from 408 to 500.

Whilst it is logically correct to use 408 for the
ErrorStatus::ScriptTimeout and ::Timeout errors, it
causes a collision with HTTP semantics implement in HTTP clients.
To support Keep-Alive we allow retries in HTTP clients and if a
client sees code 408 it thinks that the server has gone away and
retries the request.

This causes WebDriverCommand::ExecuteScript, ::Get, and ::Refresh
commands to be sent twice with some HTTP clients.

This is a backwards incompatible change to WebDriver in order to
not break HTTP/1.1 Keep-Alive.

Comment 2

[Andreas Tolfsen ❲:ato❳]

Let’s see if this works…

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ded2cfa583ebdc62bb7f5305d89d3381ca27f38a

Comment 3

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

Comment on attachment 9002753 [details] [diff] [review]
Change ErrorStatus::{ScriptTimeout,Timeout} HTTP codes from 408 to 500.

Review of attachment 9002753 [details] [diff] [review]:
-----------------------------------------------------------------

This looks fine to me. But could you please wait with landing this patch until my Serde patch landed? Otherwise it will cause merge conflicts for me again. Thanks!

Comment 4

[Andreas Tolfsen ❲:ato❳]

Yes, was going to hold off.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/72a737dd8639
Change ErrorStatus::{ScriptTimeout,Timeout} HTTP codes from 408 to 500. r=whimboo

Comment 6

[Andreas Tolfsen ❲:ato❳]

As expected there were no conflicts during rebase here.

Comment 7

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

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

Comment 8

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

Can't merge web-platform-tests PR due to failing upstream checks:
Github PR https://github.com/web-platform-tests/wpt/pull/12641
* continuous-integration/travis-ci/pr (https://travis-ci.org/web-platform-tests/wpt/builds/419595734?utm_source=github_status&utm_medium=notification)

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/72a737dd8639

Comment 10

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

Upstream PR merged