Code review comment for lp:~gz/qbzr/reliable_exception_encoding_686735

On 16/04/2011 16:48, Martin [gz] wrote:
>> Martin, I'm not quite comfortable with bare `except:`, although I
>> see you explicitly catch and re-raise others exceptions. But why do
>> you use bare except here? What kind of errors we might get there?
>> IIUC we shouldn't get UnicodeError, because we try hard to avoid
>> it. Maybe something related to error in repr? Can you explain that
>> in the comments to that except, please?
>
> So, what I'm not sure about is whether we need to be careful about
> KeyboardInterrupt under Qt, the rest of the file seemed to worry
> about it.
>
> The reasoning of the try/except formulation is as follows: * repr
> invokes __repr__ which can run arbitrary code, so can raise any
> exception at all * Just catching Exception traps KeyboardInterrupt on
> Python 2.4 * Once we're catching the classes moved to BaseException
> in 2.5 first, may as well use the bare except which happens to cover
> string exceptions on versions they're still legal
>
> If KeyboardInterrupt should be being reraised, I should probably add
> that to the try/except around commands.run_bzr too, while Python 2.3
> is still supported.
>
> Is this change based on the right revision, or should I move it to be
> daggy if you want to land it as a bug fix?

Note that a KeyboardInterrupt is very likely to happen in the
subprocess. On *nix, when you press cancel, the main process will send a
interrupt signal to the subprocess, which will cause a KeyboardInterrupt
to be raised. (On Windows we do something different.)

I think that it is best to catch all exceptions here. Essentially this
becomes the top level exception handler, and KeyboardInterrupts should
be reported to the main process like any other exception.

Gary