Opened at 2012-06-15T17:39:43Z
Last modified at 2012-12-20T16:34:49Z
#1770 new defect
always include a useful message in calls to log.err
Reported by: | davidsarah | Owned by: | |
---|---|---|---|
Priority: | normal | Milestone: | soon |
Component: | code | Version: | 1.9.1 |
Keywords: | error | Cc: | |
Launchpad Bug: |
Description
From #1768:
The "Unhandled Error" (as opposed to the usual "Unhandled Error in Deferred") means that something called twisted's log.err() with a Failure or Exception, but nothing else (in particular not a why= argument). This is a pretty common (perhaps unfortunate) pattern in our code: tahoe has roughly 47 such calls, and Foolscap has about 29. So we don't really know what caught the exception (and therefore what ought to be handling it differently).
We should fix all of these.
Change History (3)
comment:1 Changed at 2012-06-17T13:00:49Z by zooko
comment:2 Changed at 2012-06-21T22:56:50Z by warner
Hm, maybe. It'd have to look something like this:
d = ... d.addCallback(...) util.terminateDeferredChain(d) def terminateDeferredChain(d): stack = traceback.format_stack(1) d.addErrback(log.err, why="call stack was %s" % (stack,))
(since log.err is usually added to the end of a Deferred chain, by the time it's called, the stack no longer contains any information about what caused the log.err to be added)
My problem with that is twofold:
- tracebacks are expensive to create, and terminateDeferredChain() gets called every time, not just in the case where log.err() gets invoked
- I've rarely found a whole stacktrace to be significantly more useful than a UMID, at least for async errors. There just aren't very many different ways to reach the same function: the top stack frame is enough to guess the rest, and a UMID is enough to deduce the top frame.
Also, the "util.terminateDeferredChain(d)" at the end of a series of "d.addCallback" lines would look kind of out-of-place, but maybe I could get used to it.
It might be worth testing how expensive it is to extract just a single stack frame, instead of the whole traceback. If terminateDeferredChain() could grab just the file+lineno+name of the calling function, that'd contain as much information as a UMID but tolerate cut-and-paste coding a lot better. OTOH, a static UMID is super-fast.
comment:3 Changed at 2012-12-20T16:34:49Z by davidsarah
- Milestone changed from 1.10.0 to 1.11.0
Could we make a utility function that extracts the current stack trace and encodes that into the why= argument? Then lazy programmers could just call that function instead of writing a why= explanation themselves.