1833578 - [wdspec] ScriptEvaluateResultException doesn't have a string representation (missed __str__)

Status: RESOLVED FIXED

(Remote Protocol :: WebDriver BiDi, defect, P1)

Description

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

Right now we only see a generic Script execution failed error in case script.evaluate or script.callFunction are failing to execute a script. See bug 1833562 as example.

This happens because the ScriptEvaluateResultException class doesn't have a __str__ method to create human readable string and we only initialize the super class with the given error message:

https://searchfox.org/mozilla-central/rev/e77d89c414eac63a295a2124600045775a6f4715/testing/web-platform/tests/tools/webdriver/webdriver/bidi/modules/script.py#11

Also I think that this error class should actually be based off the BidiException class:

https://searchfox.org/mozilla-central/rev/e77d89c414eac63a295a2124600045775a6f4715/testing/web-platform/tests/tools/webdriver/webdriver/bidi/error.py#8-31

Comment 1

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

It would be good to know if we could add general Wdspec tests for this but then the returned error message should be the same.

I think that for specific Javascript errors as thrown by the test itself this will be fine. But if a browser related error happens we cannot align on a message format, but this is most likely ok and should not be covered by such a wdspec test.

James, would you foresee any issues with that? If not we should get this fixed / extended soon to have better failure messages in CI.

Comment 2

[James Graham [:jgraham]]

I don't think we can expect different browsers to return exactly the same error string in any situation.

Also note that this isn't a protocol-level error, but the representation of a JS exception. So it doesn't obviously make sense for it to map to BidiException; that is currently only for actual protocol errors.

Comment 3

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

Thanks. And yes, that makes sense. In that case lets just add a __str__ method to the class.

Comment 4

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

Attachment: Bug 1833578 - [wdspec] Added string representation for ScriptEvaluateResultException.

Comment 5

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

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

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/e4245b1c6a2b
[wdspec] Added string representation for ScriptEvaluateResultException. r=webdriver-reviewers,jgraham

Comment 7

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/e4245b1c6a2b

Comment 8

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

Upstream PR merged by moz-wptsync-bot

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e8c7f34af62a
[wpt PR 40443] - [Gecko Bug 1833578] [wdspec] Added string representation for ScriptEvaluateResultException., a=testonly

Comment 10

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/e8c7f34af62a