This has the benefit that we now get the usual data about the
user/request/etc. in error emails related to bugdown exceptions;
previously we were just getting the traceback in the emails (since our
`mail_admins` template was very simple) and no other debugging
details.
Comments tweaked by tabbott to help make clear exactly what's going on
here, since it's a little subtle and a little hacky.
Fixes#8843.
This cuts out about 11 calls to `git describe`. In a nice fast LXC
container following our instructions for development on a Linux host,
this might save "only" about 1.5s; in a dev environment on a Windows
host, the savings have been clocked at 49s, presumably due to an
extremely slow filesystem in the VM.
The tests weren't doing much with this codepath as they were, and
there isn't a lot of value to be gained by testing it anyway; it's
totally non-critical and rarely changes.
[Commit message rewritten by greg.]
This is nicer than the "For manual testing ..." comment. :-)
Also as a proper setting we can have it control some logging I
added locally while testing my recent changes to pika logging.
When I added this "Deployed code" feature to the error reporting,
I apparently hadn't worked out enough of how this code works to
realize that `notify_server_error` may be in a different process,
at a different time and potentially even on a different machine
from the actual error being reported.
Given that architecture, all the data about the error must be computed
in `AdminNotifyHandler`, before sending the report through the queue,
or else it risks being wrong. The job of `notify_server_error` and
friends is only to format the data and send it off. So, move the
implementation of this feature in order to do that.
(@showell added some "nocoverage" directives here for code that
is hard to test (exceptions being thrown, deployment files not
existing) and that was originally part of a file that didn't
require 100% coverage)
This helps prevent them from diverging and getting different sets of
features and fixes. As a bonus, the email path gets a nice tweak that
the Zulip path has had for years, since f7f2ec0ac, which makes the
emails clearer and less broken-looking when logging a message with no
stack trace.
This deduplicates a little bit of logic, and also has us always put
things into `report` the same way.
Empirically an exception in this codepath is very rare, so we won't
complicate the code by trying to salvage a lot of partial information
if it happens -- just log the traceback, and try to get a minimal
notification sent of the bare fact this happened.
This name hasn't been right since f7f2ec0ac back in 2013; this handler
sends the log record to a queue, whose consumer will not only maybe
send a Zulip message but definitely send an email. I found this
pretty confusing when I first worked on this logging code and was
looking for how exception emails got sent; so now that I see exactly
what's actually happening here, fix it.
Anders Kaseorg
Shubham Dhama
Aayush Agrawal
Greg Price
Steve Howell
rht
Robert Hönig
Tim Abbott
Tommy Ip
Alex Huang
Tim Abbott