Opened 12 years ago

Closed 11 years ago

#1403 closed defect (fixed)

shell breaks if system bus cannot be accessed

Reported by: sascha_silbe Owned by: sascha_silbe
Priority: Unspecified by Maintainer Milestone: Unspecified
Component: Sugar Version: Git as of bugdate
Severity: Major Keywords: r+
Cc: Distribution/OS: Unspecified
Bug Status: New

Description

Similar to #1401, the shell breaks if the system bus is unreachable, even though it's only required for optional features (non-permanent connection to jabber server, removable media, etc.):

Traceback (most recent call last):
  File "/home/sascha.silbe/sugar-jhbuild/install/bin/sugar-session", line 225, in <module>
    main()
  File "/home/sascha.silbe/sugar-jhbuild/install/bin/sugar-session", line 206, in main
    home_window = homewindow.get_instance()
  File "/home/sascha.silbe/sugar-jhbuild/install/lib/python2.5/site-packages/jarabe/desktop/homewindow.py", line 183, in get_instance
    _instance = HomeWindow()
  File "/home/sascha.silbe/sugar-jhbuild/install/lib/python2.5/site-packages/jarabe/desktop/homewindow.py", line 63, in __init__
    self._mesh_box = MeshBox()
  File "/home/sascha.silbe/sugar-jhbuild/install/lib/python2.5/site-packages/jarabe/desktop/meshbox.py", line 778, in __init__
    netmgr_observer = NetworkManagerObserver(self)
  File "/home/sascha.silbe/sugar-jhbuild/install/lib/python2.5/site-packages/jarabe/desktop/meshbox.py", line 656, in __init__
    self._bus = dbus.SystemBus()
  File "/usr/lib/pymodules/python2.5/dbus/_dbus.py", line 202, in __new__
    private=private)
  File "/usr/lib/pymodules/python2.5/dbus/_dbus.py", line 108, in __new__
    bus = BusConnection.__new__(subclass, bus_type, mainloop=mainloop)
  File "/usr/lib/pymodules/python2.5/dbus/bus.py", line 125, in __new__
    bus = cls._new_for_bus(address_or_type, mainloop=mainloop)
dbus.exceptions.DBusException: org.freedesktop.DBus.Error.FileNotFound: Failed to connect to socket /var/run/dbus/system_bus_socket: No such file or directory

Attachments (1)

0001-fail-gracefully-if-system-DBus-unreachable-1403.3.patch (8.6 KB) - added by sascha_silbe 12 years ago.
fail gracefully if system DBus unreachable

Download all attachments as: .zip

Change History (12)

comment:1 Changed 12 years ago by sascha_silbe

  • Keywords r? added
  • Owner changed from tomeu to sascha_silbe
  • Status changed from new to accepted

Again I need to test on a system with a working system bus, but we can start review anyway. The patch is a bit large because of whitespace changes (i.e. indentation), but the actual number of changes is quite small.

Changed 12 years ago by sascha_silbe

fail gracefully if system DBus unreachable

comment:2 Changed 12 years ago by tomeu

Can you please explain how applying this patch will help our users?

While degrading functionality gracefully can be seen as desirable, it increases the support costs because makes harder understand why something isn't working.

And right now we have more coders than testers and bug triagers.

comment:3 follow-up: Changed 12 years ago by sascha_silbe

It helps with testing in chroots. As you said yourself we need more testers, so IMO we should make it easier for them.
ARM (or at least the ARM based systems I've used so far) doesn't have virtualization, so chroots are the only way to test without affecting the regular system.

comment:4 in reply to: ↑ 3 ; follow-up: Changed 12 years ago by tomeu

Replying to sascha_silbe:

It helps with testing in chroots. As you said yourself we need more testers, so IMO we should make it easier for them.

Agreed, but shouldn't we be testing on an environment closer to that of our users?

comment:5 in reply to: ↑ 4 ; follow-up: Changed 12 years ago by sascha_silbe

Replying to tomeu:

Agreed, but shouldn't we be testing on an environment closer to that of our users?

Where possible we should, of course. But the alternative here is no testing at all on some systems.
I'd like to see as much testing as possible on all levels, from automated (unit/regression) tests up to manual testing on "real" systems (not KVM/qemu). The more bugs we catch on lower levels, the less work our human testers need to do.

The test suite for datastore (will be submitted for review soon) and the patch we're talking about already enabled me to find (and fix) several bugs. IMO that's well worth it.
Unless there's a bug in my patch, the only effect will be that instead of all of Sugar not working, only the features relying on the system bus will be disabled. That's easily recognizable and the exceptions are always in the logs, so discovering the reason for it isn't hard either. Reading logs is already required if the system bus is reachable, but the individual services are not, so to the (non-chroot) user it doesn't make a real difference.
Even if there is a bug in my patch, that's well ofset by the bugs it enabled me to find. :)

comment:6 in reply to: ↑ 5 ; follow-up: Changed 12 years ago by tomeu

Replying to sascha_silbe:

Replying to tomeu:

Agreed, but shouldn't we be testing on an environment closer to that of our users?

Where possible we should, of course. But the alternative here is no testing at all on some systems.

So we cannot run or connect to the system bus on those systems?

I'd like to see as much testing as possible on all levels, from automated (unit/regression) tests up to manual testing on "real" systems (not KVM/qemu). The more bugs we catch on lower levels, the less work our human testers need to do.

The test suite for datastore (will be submitted for review soon) and the patch we're talking about already enabled me to find (and fix) several bugs. IMO that's well worth it.
Unless there's a bug in my patch, the only effect will be that instead of all of Sugar not working, only the features relying on the system bus will be disabled. That's easily recognizable and the exceptions are always in the logs, so discovering the reason for it isn't hard either. Reading logs is already required if the system bus is reachable, but the individual services are not, so to the (non-chroot) user it doesn't make a real difference.
Even if there is a bug in my patch, that's well ofset by the bugs it enabled me to find. :)

I don;t think it's a big deal in this concrete patch, but if we need to make all Sugar features "degrade gracefully", do you see the amount of code we'll need to change and add? That's why I'm asking these questions.

comment:7 in reply to: ↑ 6 ; follow-up: Changed 12 years ago by sascha_silbe

Replying to tomeu:

So we cannot run or connect to the system bus on those systems?

I guess it's possible to set up a separate system bus inside the chroot, but it's non-trivial AFAICT and complicates testing.

I don;t think it's a big deal in this concrete patch, but if we need to make all Sugar features "degrade gracefully", do you see the amount of code we'll need to change and add? That's why I'm asking these questions.

Sure. I would be doing the same. :)

comment:8 in reply to: ↑ 7 ; follow-up: Changed 12 years ago by tomeu

Replying to sascha_silbe:

Replying to tomeu:

So we cannot run or connect to the system bus on those systems?

I guess it's possible to set up a separate system bus inside the chroot, but it's non-trivial AFAICT and complicates testing.

And what about connecting to the external bus? Is it possible?

comment:9 in reply to: ↑ 8 Changed 12 years ago by sascha_silbe

Replying to tomeu:

And what about connecting to the external bus? Is it possible?

Even if there was one, i would need to dive into dbus to figure out how to prevent the chroots from doing bad things on the real system. It might not even be possible.

comment:10 Changed 11 years ago by tomeu

  • Keywords r+ added; r? removed

r+ with the understanding that this will enable automatic tests and that Sugar won't be more expensive to support because of the reduced feedback when such serious failures happen. Please push, thanks!

comment:11 Changed 11 years ago by sascha_silbe

  • Resolution set to fixed
  • Status changed from accepted to closed

Rebased (much simpler now thanks to dsd's refactorings), tested and pushed as 08b278a. Thanks!

Note: See TracTickets for help on using tickets.