search

Brewtarget / brewtarget

Main brewtarget source code repository.
GNU General Public License v3.0
315 stars 134 forks source link

Adding yeast to a recipe crashes when using postgresql #508

Closed mikfire closed 2 years ago

mikfire commented 3 years ago

I get

The application encountered a fatal error.
Error message:
could not execute query: SELECT yeast_id as id FROM yeast_in_recipe WHERE recipe_id = 127 : ERROR:  current transaction is aborted, commands ignored until end of transaction block
(25P02) QPSQL: Unable to create query

Those are hard to track down.

mikfire commented 3 years ago

I am getting a similar error when adding hops too.

Error message:
could not execute query: SELECT hop_id as id FROM hop_in_recipe WHERE recipe_id = 128 : ERROR:  current transaction is aborted, commands ignored until end of transaction block
(25P02) QPSQL: Unable to create query
matty0ung commented 3 years ago

Seems like a query barfed inside a transaction and that's preventing subsequent queries running (until transaction is closed). Can you get any details of the previous DB query, from either Brewtarget logs or PostgreSQL logs?

mikfire commented 3 years ago

So a little digging last night and what had happened is I was connecting to an older database that didn't have the inventory_id field in the right places. This caused the initial load of the database to throw A LOT of errors.

It is completely self-inflicted, as I have too many databases laying around in unknown, unremembered states of disrepair. I doubt anybody but me would have ever had this exact error happen. And there will be a culling soon,

However. It seems to me that the actual error message happened far too far away from where the error happened. If I hadn't gotten in the habit of always running with the postgresql logging enabled, and if I hadn't thought to look at them, I would have never figured out the issue.

So I want to use this as an opportunity to clean that up and make it fail better. One question. In previous swings at this, I had simply thrown errors up the stack until I ran out of stack. Lately, I've become a real fan of calling abort() in the catch. If run under gdb, it provides a stack trace that gets you fairly close to where the error happened.

Any suggestions or thoughts on the "right" way to handle this?

matty0ung commented 3 years ago

It's a good question. I think abort() is a last resort to call if we can't continue and can't exit gracefully (with some helpful message to the user). If there are circumstances where we can convince ourselves "an end user is never going to see this problem", then an assert is a clear way of stating this and gets us the core file & stack trace. Otherwise, log as much info as possible and get far enough back up the stack (eg by throwing another exception) so we can tell the user "Sorry, we hit a fatal error, but here's what to do / where to look for more info".

AIUI there are ways to have our cake and eat it - ie log a stack trace without calling abort()/assert() - though it's a bit OS-dependent: on Linux, use backtrace() & backtrace_symbols(). This code https://gist.github.com/fmela/591333 gives the rough idea, although personally I'd rewrite it as it looks too much like C to me!

matty0ung commented 2 years ago

I'm going to close this specific ticket as the crash is not a general issue - just if you connect to an out-of-date DB as you say.

On the point about reporting errors, we've added Boost stacktrace to the build as it can, in principle, be used to give us stack traces in the logs, but there's still a bit of fiddling around to get it to work on all platforms.