Can no longer catch socket.error in marionette client

RESOLVED FIXED in Firefox 23

Status

Testing
Marionette
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: kumar, Assigned: ahal)

Tracking

unspecified
mozilla23
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(firefox21 wontfix, firefox22 wontfix, firefox23 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix)

Details

Attachments

(1 attachment)

As of the Python marionette_client version 0.9 I can no longer catch socket.error and this makes programmatic access difficult.

This is the culprit around line 400 of marionette.py:

    def start_session(self, desired_capabilities=None):
        try:
            # We are ignoring desired_capabilities, at least for now.
            self.session = self._send_message('newSession', 'value')
        except:
            traceback.print_exc()
            self.check_for_crash()
            sys.exit()

Why not catch and re-raise like this?

    def start_session(self, desired_capabilities=None):
        try:
            # We are ignoring desired_capabilities, at least for now.
            self.session = self._send_message('newSession', 'value')
        except:
            exc, val, tb = sys.exc_info()
            self.check_for_crash()
            raise exc, val, tb

My script was catching a socket.error and reacting by forwarding ports: https://github.com/kumar303/ezboot/blob/70fb41fe53ee17d377424065706909651e037d60/ezboot/__init__.py#L120-L124
the workaround is to catch SystemExit which is fragile
Assignee: nobody → ahalberstadt
(Assignee)

Comment 2

5 years ago
Created attachment 739639 [details] [diff] [review]
Patch 1.0 - fix

Good catch, I forgot you could raise like that. Kumar, do you need this on b2g18 too?
Attachment #739639 - Flags: review?(jgriffin)
Comment on attachment 739639 [details] [diff] [review]
Patch 1.0 - fix

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

Thanks!
Attachment #739639 - Flags: review?(jgriffin) → review+
https://hg.mozilla.org/mozilla-central/rev/eb2202cf3489
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Thanks Andrew. I don't think it matters if this lands on b2g18 or not. Just let me know when a new version of the lib is available on PyPI and when gaiatest has been updated to depend on it.
(Assignee)

Comment 7

5 years ago
(In reply to Kumar McMillan [:kumar] from comment #6)
> Thanks Andrew. I don't think it matters if this lands on b2g18 or not. Just
> let me know when a new version of the lib is available on PyPI and when
> gaiatest has been updated to depend on it.

If this is blocking something we can bump the version specifically for this. Otherwise we'll just wait until there's a need to.
It's not blocking, we can work around by catching SystemExit
A new version of marionette has just been released on pypi (version 0.5.25), but can this be uplifted to b2g18? Best to keep things in sync so we don't go hunting for bugs when the next conflict happens!
Keywords: checkin-needed
https://hg.mozilla.org/releases/mozilla-b2g18/rev/14b6336f254d
status-b2g18: --- → fixed
status-b2g18-v1.0.0: --- → wontfix
status-b2g18-v1.0.1: --- → wontfix
status-firefox21: --- → wontfix
status-firefox22: --- → wontfix
status-firefox23: --- → fixed
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.