(gtk3-only) unhandled exception when importing optional modules

Bug #1574399 reported by Emilio Pozuelo Monfort on 2016-04-24
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Terminator
Undecided
Unassigned

Bug Description

Commit 1618 added gi.require_version() calls, which throw a ValueError if the requested version is not available. However the except clause is only catching ImportErrors.

The attached patch catches all exceptions, fixing this crash.

Related branches

Emilio Pozuelo Monfort (pochu) wrote :
Emilio Pozuelo Monfort (pochu) wrote :

Any chance this can get merged in the gtk3 branch?

Stephen Boddy (stephen-j-boddy) wrote :

Can you explain why this is wrong?

If you don't have the right version, then at least the config.py and window.py *should* throw an exception and prevent running.

Yes, the activity watch plugin should probably catch the import error, and just not make the plugin available. That can be done by simply adding ValueError to the exception clause, i.e.
except (ImportError, ValueError):

That would catch both, and prevent Terminator bugging out if the wrong version of the library is available.

Changed in terminator:
status: New → Incomplete
Emilio Pozuelo Monfort (pochu) wrote :

You are trying to make some modules optional, e.g. GConf. So if the package is not available, terminator should still work.

We used to have:

try:
    import gi
    from gi.repository import GConf
except ImportError:
    dbg('Unable to import gconf, GNOME defaults unavailable')

If gi or GConf are not available, that throws an ImportError, which you catch and handle, so everything is fine, and you continue to run without GConf.

However, we now have:

try:
    import gi
    gi.require_version('GConf','2.0')
    from gi.repository import GConf
except ImportError:
    dbg('Unable to import gconf, GNOME defaults unavailable')

If you don't have GConf, that will fail on gi.require_version, which will throw a ValueError. Since ValueError is not handled, we crash:

emilio@tatooine:~/src/terminator-gtk3$ ./terminator
Traceback (most recent call last):
  File "./terminator", line 47, in <module>
    import terminatorlib.optionparse
  File "/home/emilio/src/terminator-gtk3/terminatorlib/optionparse.py", line 25, in <module>
    import config
  File "/home/emilio/src/terminator-gtk3/terminatorlib/config.py", line 84, in <module>
    gi.require_version('GConf','2.0')
  File "/usr/lib/python2.7/dist-packages/gi/__init__.py", line 118, in require_version
    raise ValueError('Namespace %s not available' % namespace)
ValueError: Namespace GConf not available

Handling both ImportError and ValueError explicitly would be fine as well indeed.

Hope that's clearer.

Emilio Pozuelo Monfort (pochu) wrote :

Ping? :)

Stephen Boddy (stephen-j-boddy) wrote :

Committed revision 1726.

Yup when I sat and properly read it, you made sense. Minor diff in that config.py no longer needs changing, because we only used GConf to get the system fonts. In gtk3 those values are in DConf and (as per usual with gtk) the steps are completely different.

Changed in terminator:
status: Incomplete → Fix Committed
milestone: none → 2.0
Changed in terminator:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers