search

oxidecomputer / async-bb8-diesel

Safe asynchronous access to Diesel and the bb8 connection manager
MIT License
12 stars 8 forks source link

wrong message on PoolError::Connection #6

Closed davepacheco closed 3 years ago

davepacheco commented 3 years ago

The "thiserror" message attached to PoolError::Connection: https://github.com/oxidecomputer/async-bb8-diesel/blob/1a5c55d9e31210fd7dfff944798e89578e3a0dfc/src/error.rs#L32

says "Failed to checkout a connection". But that variant is also used for ordinary query errors. I don't think we can hit this today in Omicron because we don't print the PoolError's message directly, but this could send somebody on a wild goose chase if we ran into it in the future.

smklein commented 3 years ago

I'll update it - but I think it'll still be a little vague, because it's currently "everything except a bb8 timeout".

Does Failure accessing a connection sound better?

diesel::r2d2::Error includes failure to establish / ping the connection. diesel::result::Error looks like it includes most errors from the db itself

davepacheco commented 3 years ago

As I understand it, it's quite possible for the error to have nothing to do with the connection at all. It could be a SQL syntax error, for example. I do think the updated message is less likely to send people down the wrong path. Thanks!

smklein commented 3 years ago

yeah, I tried to make it "failure while accessing the connection" to imply that "could be a lot of things, but you had the connection in hand when it fell over". Additional wordsmithing welcome.