Closed
Bug 965782
Opened 11 years ago
Closed 11 years ago
Exception marionette.errors.InvalidResponseException: InvalidResponseException()
Categories
(Remote Protocol :: Marionette, defect)
Tracking
(b2g-v1.3 fixed)
RESOLVED
FIXED
mozilla29
Tracking | Status | |
---|---|---|
b2g-v1.3 | --- | fixed |
People
(Reporter: cbook, Assigned: mdas)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
mdas
:
review+
|
Details | Diff | Splinter Review |
noticed in some recent timeout testfailures for b2g the error
Exception marionette.errors.InvalidResponseException: InvalidResponseException() in <bound method Marionette.__del__ of <marionette.marionette.Marionette object at 0x1f7e3d0>> ignored
examples are
https://tbpl.mozilla.org/php/getParsedLog.php?id=33807507&tree=Mozilla-Inbound
and
https://tbpl.mozilla.org/php/getParsedLog.php?id=33808637&tree=Fx-Team
not sure what this is, maybe a regression ?
Assignee | ||
Comment 1•11 years ago
|
||
These are showing up in Mochitests with the pattern:
05:46:30 INFO - 97351 INFO TEST-PASS | /tests/layout/style/test/test_value_cloning.html | computed values should match when cloning -moz-transform-origin: calc(20px + 50%) calc(50% - 10px)
05:46:30 INFO - 97352 INFO TEST-PASS | /tests/layout/style/test/test_value_cloning.html | serialization should match when cloning -moz-transform-origin: calc(20px + 1em) calc(20px / 2)
05:52:00 INFO - 9735B2GRunner TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_value_cloning.html | application timed out after 330.0 seconds with no output
05:52:01 INFO - B2GRunner INFO | checking for crashes in '/data/local/tests/profile/minidumps'
05:53:10 INFO - Mochitest INFO | runtestsb2g.py | Running tests: end.
05:56:11 INFO - 3 INFO TEST-PASS | /tests/layout/style/test/test_value_cloning.html | computed values should match when cloning -moz-transform-origin: calc(20px + 1em) calc(20px / 2)
05:56:11 INFO - 97354 INFO TEST-PASS | /tests/layout/style/test/test_value_cloning.html | serialization should match when cloning -moz-transform-origin: calc(20px) calc(20px)
05:56:11 INFO - 97355 INFO TEST-PASS | /tests/layout/style/test/test_value_cloning.html | computed values should match when cloning -moz-transform-origin: calc(20px) calc(20px)
05:56:11 INFO - Exception marionette.errors.InvalidResponseException: InvalidResponseException() in <bound method Marionette.__del__ of <marionette.marionette.Marionette object at 0x165c3d0>> ignored
05:56:11 ERROR - Return code: 247
05:56:11 INFO - dumping logcat
Ie: a test times out, some tests run after that, and then marionette has trouble.
Assignee | ||
Comment 2•11 years ago
|
||
ahal suggested https://github.com/mozilla/mozbase/blob/master/mozrunner/mozrunner/remote.py#L209 and https://github.com/mozilla/mozbase/blob/master/mozrunner/mozrunner/remote.py#L332 as likely areas to look into
Comment 3•11 years ago
|
||
Many of the logs being pasted in bug 965677 have this failure present.
Blocks: 965677
Assignee | ||
Comment 4•11 years ago
|
||
delete_session is useful if the session is still active and can be used. There are times when it isn't (socket closed, gecko process crashed, marionette crashed, timeout errors etc.), and calling it will just generate one of many kinds of errors.
I've modified the code so we simply try to delete the session if possible or ignore the error silently, since it is not crucial to delete the session here and the error is not useful on its own. Once the marionette object is deleted, the current connection will be closed anyway, and if the marionette server is still running, then it will accept a new connection for the next test.
The current behaviour is to throw the error and it's been causing some problems in the tests.
Attachment #8368079 -
Flags: review?(wlachance)
Comment 5•11 years ago
|
||
Comment on attachment 8368079 [details] [diff] [review]
don't throw exceptions when trying to cleanup
Agree with the general idea but I think we should be more explicit about which types of exceptions we want to ignore. Also, we should have a comment here describing the logic.
Attachment #8368079 -
Flags: review?(wlachance) → review-
Assignee | ||
Comment 6•11 years ago
|
||
I would agree with you, but there aren't any Exceptions here that we do want to bubble up, unless you can think of one?
The idea here is to just try to delete the session to do server side cleanup. If we can't successfully do that, it doesn't really matter to cleanup() or whoever is deleting the object, since we can't assume that the marionette server will be running (or in a good state) when we're done with the Marionette object.
Comment 7•11 years ago
|
||
(In reply to Malini Das [:mdas] from comment #6)
> I would agree with you, but there aren't any Exceptions here that we do want
> to bubble up, unless you can think of one?
Even syntax errors? :) A blanket try...except allows code like this to fail silently:
>>> try:
... bldka
... except Exception:
... pass
...
>>>
I really frown on this because it can mask problems in your code. I would just enumerate out the failure possibilities and catch all of them.
Assignee | ||
Comment 8•11 years ago
|
||
ok, I should note that the exceptions thrown here will be varied, depending on the problem (InvalidResponseException, socket.timeout, some other stuff I saw locally but can't remember) so if we want to whitelist them, the list won't be complete until we let this out in the wild and add any upcoming exception
Assignee | ||
Comment 9•11 years ago
|
||
Attachment #8368079 -
Attachment is obsolete: true
Attachment #8368094 -
Flags: review?(wlachance)
Comment 10•11 years ago
|
||
Comment on attachment 8368094 [details] [diff] [review]
marioerr
Please add a comment explaining why we ignore these exceptions. r+ with that change.
Attachment #8368094 -
Flags: review?(wlachance) → review+
Assignee | ||
Comment 11•11 years ago
|
||
posting updated patch, moving r+ forward.
landed in m-i:
https://hg.mozilla.org/integration/mozilla-inbound/rev/73e4adef1599
Attachment #8368094 -
Attachment is obsolete: true
Attachment #8368114 -
Flags: review+
Reporter | ||
Comment 12•11 years ago
|
||
Assignee: nobody → mdas
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Assignee | ||
Updated•11 years ago
|
status-b2g-v1.3:
--- → fixed
Updated•2 years ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•