The provider type was modelled as a newtype struct around a `String` but
there's a finite number of values, so really it should be an enum. This
wasn't too pressing until now but will shortly come in useful for the
configuration stuff, which needs a provider type too.
The error-handling in this repo grew pretty organically without us
giving much thought to an over-arching strategy. The end result of that
was a proliferation of different error types that weren't really adding
much value.
The API here is simple and the 4xx responses are really limited to bad
requests and bounce/complaint limit violations. Everything else is an
internal server error. As such, the `error` module could be simplified
by employing a blanket `Internal` error type to cover the multitude of
niche errors that should never occur during normal conditions. The
detail for those errors is still available in the `message` property and
Sentry will still log the backtrace of course.
I also noticed that the Rocket catcher stuff wasn't really being used,
so pulled that out too. We have a standard JSON error format like the
rest of the FxA ecosystem, and we always want the response payload to be
the serialisation of that structure.
One side-effect of all these changes is that the errno value has changed
in most cases. However, I took care to preserve the errno for the bounce
and complaint violations, because it's hard-coded in the auth server. It
wouldn't have been that onerous to open a PR for commensurate changes
over there, but the number of errors left here worked out in such a way
that made sense not to bother.
Fixes#201.
Email addresses that we receive from bounce, complaint and delivery
notifications may not be identical matches to what we set on messages.
We already did some normalization by lower-casing the address when
parsing. This change extends that normalization process by trimming
whitespace and unwrapping the address part if it has been wrapped in
angle bracket delimiters to separate it from the display name. This
matches the behaviour of the auth server.
We were sending errors to Sentry at the point where they are logged,
but there are a number of paths where an error is raised without being
logged. Instead, this change captures them at the point of creation,
which should mean we now capture every error.
Fixes#205.
SES provides a UTC-based timestamp for bounce and complaint events but
the auth server opted to ignore it. I'm not sure that was the optimal
decision because messages might linger on a queue (either ours or the
provider's) for a length of time before we process it. Given that we
already trust SES et al to deliver sane notifications, it doesn't seem
too great a stretch to also trust the timestamp on those notifications.
Phil Booth
Vlad Filippov
Vijay Budhram