apport Python exception hook too aggressive

Indeed the current hook already does not create a report for scripts in your home directory:

        if not apport.fileutils.likely_packaged(binary):
            return

which returns true if the executable path starts with the usual system directories (/usr/bin, /lib, etc.).

So, if I understood you correctly, 'import apport.report' is too heavy here? The code can certainly be reshuffled to postpone that import until after above check, which would reduce the amount of parsed code for an exception in non-packaged Python programs.

 apport (0.81) gutsy; urgency=low
 .
   * apport/report.py: Remove '[apport]' default bug title prefix. (LP: #94819)
   * apport/crashdb_impl/launchpad.py: Tag new bugs with
     'apport-<problemtype>'. This replaces the former '[apport]' prefixing.
   * debian/local/setup-apport-retracer: Specify a path in '.' command and
     use sh again. Yay for me needing three attempts before actually RTFMing
     how '.' works (which is really nasty and strange IMHO).
   * bin/apport-chroot: Fix symlinks before repackaging the chroot tarball in
     'install' and 'installdeb' modes.
   * debian/local/setup-apport-retracer: Install python-libxml2 and python-apt.
   * bin/launchpad-crash-digger: Supply --auth instead of the deprecated
     --cookie to apport-chroot.
   * bin/apport-chroot: Fix identifier name in command_retrace().
   * debian/local/setup-apport-retracer: Set APPORT_CRASHDB_CONF to the local
     crashdb.conf.
   * bin/apport-chroot: Unset APPORT_CRASHDB_CONF for login and retrace.
   * bin/launchpad-crash-digger: Check the release of a bug and whether we have
     a chroot for it before untagging it. This avoids loosing tags for bugs we
     do not yet have a working retracer chroot for.
   * bin/apport-retrace: Do not abort with an exception if package installation
     fails. Give a proper error message instead and point to -u. (LP: #115681)
   * apport/crashdb_impl/launchpad.py, update(): Create a temporary directory
     and use proper file names for the new attachments. With TemporaryFile(),
     attachment file names ended up as '<fdopen>'. (LP: #115347)
   * apport/report.py, add_os_info(): Add field 'NonfreeKernelModules' which
     lists loaded kernel modules which do not have a FOSS license. This is
     particularly helpful for quickly checking for restricted graphics drivers.
     (LP: #103239)
   * apport_python_hook.py: Move the apport.* imports into the try: block and
     move the likely_packaged() test to the top, to avoid importing
     apport.report and creating a Report object for non-packaged scripts. This
     makes the entire code more efficient and robust against errors in the
     apport modules. (LP: #109955)
   * apport/report.py, add_gdb_info(): Intercept OSError from gdb invocation
     (which might be segfaulting itself) and just do not put any gdb output
     into the report. The automatic retracers can try their luck again.
     (LP: #112501)
   * bin/apport-retrace: Fix handling of packages which are still known to
     /var/lib/dpkg/status, but do not have an apt record any more; treat them
     like virtual packages and just issue a warning instead of falling over.
     (LP: #107474)

The problem I ran into didn't happen during import-time (probably because the python 2.4 ctypes bug was already fixed by that time) but during run-tme. It happened because 'any()' was used in the report code, triggered by the line pr.add_proc_info(), which already is using the report module. This means the buggy report module was being used before the fileutils.likely_packaged check was even reached.

if you reshuffled the code to make the likely_packaged() check happen before any report code is called at all, that would be best. It's important an absolute minimum happens before the system decides not to do anything, because the more work it does, the more likely it is there is a bug. Glancing at the code such reshuffle looks possible, so I hope that is what you committed.

Hi,

Martijn Faassen [2007-05-24 10:27 -0000]:
> The problem I ran into didn't happen during import-time (probably
> because the python 2.4 ctypes bug was already fixed by that time) but
> during run-tme. It happened because 'any()' was used in the report code,

Right, that was fixed a while ago in feisty-updates as well.

> if you reshuffled the code to make the likely_packaged() check happen
> before any report code is called at all, that would be best.

Right, that's what is happening now. apport.report is not even
imported when likely_packaged() fails.