915595 - Allow execute_async_script to silently log JavaScript exceptions

Status: RESOLVED WONTFIX

(Remote Protocol :: Marionette, defect)

Description

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Currently the execute_async_script command throws any JavaScript exceptions that occur before the script returns. This often causes a problem when restarting B2G devices as such exceptions can be common while we're asynchronously waiting for the FTU/Homescreen app to be loaded.

Catching these exceptions has helped to identify regressions, however this is often at the cost of any test coverage on that build. What we'd like is to be able to catch these exceptions and log them so they can be reviewed after the tests have completed.

Comment 1

[Malini Das [:mdas] - Away, not checking bugmail]

I spoke with Dave and adding a way to ignore exceptions while logging them will help here.

While this is feasible, that's not to webdriver standard, nor is it something I'd very much like to advocate. It seems this should be handled by a try/catch block, so you can assess if the caught exception is what you expect, or a real error, and might lead to hiding real errors...

Comment 2

[David Burns :automatedtester]

(In reply to Malini Das [:mdas] from comment #1)
> I spoke with Dave and adding a way to ignore exceptions while logging them
> will help here.

+1. Actually I don't think it should be in logging nor in WebDriver/Marionette but its not *that* bad in logging so is the lesser of the evils. This has actually come up in https://groups.google.com/d/msg/selenium-developers/l0Kv0R7x6gk/rj44A0WlhfMJ which some would like to see in the spec. I suggest reading and maybe throwing some thoughts into that thread.

> 
> While this is feasible, that's not to webdriver standard, nor is it
> something I'd very much like to advocate. 

+1 Fail fast ftw and close all the trees if need be!

> It seems this should be handled by
> a try/catch block, so you can assess if the caught exception is what you
> expect, or a real error, and might lead to hiding real errors...

Most definitely, the minute we add something like this we are going to have egg on our face when we miss a real error.

Comment 3

[Dave Hunt [:davehunt] [he/him] ⌚BST]

That was my initial response too. It may help if I link the original discussion: https://github.com/mozilla/gaia-ui-tests/pull/1348

The problem is that this slips through TBPL because of the way we restart on desktop (because desktop is much quicker) and only affects devices. These are nightly builds, and even if the error is reported and fixed quickly the team still has to wait for a new build, which might also fail.

So really we need to find a way to not lose so much testing time to such failures.

Comment 4

[David Burns :automatedtester]

I would want us to _always_ catch the error and put it somewhere meaningful. The logging API for marionette seems like a good place to put it but we still need a mechanism to fail if there are JS errors. I would be extremely worried that we would ignore it and then potential regressions would be missed.

:davehunt mentioned that TBPL didnt catch these errors since we dont use the emulator there, maybe we should push for that rather than hiding things that might actually be useful even if caught by accident.

Comment 5

[Dave Hunt [:davehunt] [he/him] ⌚BST]

There's no guarantee that we'd catch these in TBPL when running against emulators, but I agree that's probably the best approach we have for now. This is being covered by bug 916368 but I've been really busy with performance requests recently. I hope to return to it soon.

Comment 6

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Looks like logging/parsing the logcat for JavaScript exceptions will happen in bug 959493. This should hopefully prevent the need for ignoring these exceptions.