audit use of internal_error in datastore

[davepacheco]

There are a number of functions inside DataStore that do something like this:

            some_diesel_stuff
            .execute_async(self.pool())
            .await
            .map_err(|e| {
                Error::internal_error(&format!(
                    "something bad: {:?}",
                    e
                ))
            })?;

This code always produces a 500 Internal Server Error on failure, which is not correct. If the database is down or we can't connect to it, we should be producing a 503 Service Unavailable. This sounds nitty but it's critical because clients know to retry 503s, but 500s indicate bugs and should not be retried. So this can be the difference between surviving transient failures or automatically recovering from outages and ... not doing those things.

This code is correct, though loses some useful information from the internal error message:

            some_diesel_stuff
            .execute_async(self.pool())
            .await
            .map_err(|e| {
                public_error_from_diesel_pool(e, ErrorHandler::Server)
            })?;

To preserve that context, we've talked about something analogous to anyhow::Context for our own Error type. Something like: .internal_message_context that allows you to take any Result<T, Error> and prepend the "internal_message" field of the error with whatever context you give it. Then it might look like this:

            some_diesel_stuff
            .execute_async(self.pool())
            .await
            .map_err(|e| {
                public_error_from_diesel_pool(e, ErrorHandler::Server).internal_message_context("something bad")
            })?;