Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deleting fermentable from recipe causes a crash. #436

Closed
jawsdaws opened this issue Oct 15, 2018 · 7 comments · Fixed by #450
Closed

Deleting fermentable from recipe causes a crash. #436

jawsdaws opened this issue Oct 15, 2018 · 7 comments · Fixed by #450

Comments

@jawsdaws
Copy link
Contributor

Deleting fermentable from recipe causes a crash. I bet this has something to do with 8cef99c

screenshot from 2018-10-15 17-35-50

Qt has caught an exception thrown from an event handler. Throwing
exceptions from an event handler is not supported in Qt.
You must not let any exception whatsoever propagate through Qt code.
If that is not possible, in Qt 5 you must at least reimplement
QCoreApplication::notify() and catch all exceptions there.

@pricelessbrewing
Copy link
Contributor

#435

@jawsdaws
Copy link
Contributor Author

This doesn't have anything to do with #435 . I haven't pulled those changes yet.

@bcbnz
Copy link
Contributor

bcbnz commented Jan 9, 2019

Looks like this is my fault. It works fine in c93bf1f, but not in eaf3d01 which is where PR #432 was merged in. The diff between these commits is rather small, and I can't see anything obvious which would be causing this. Admittedly I am completely unfamiliar with the workings of the database code.

Note that the delete works for fermentables in the recipes that ship with Brewtarget. It only fails on fermentables you add, which suggests the add is failing to populate all the tables in the database.

I'm not sure how much time I'll have to dig into this. One option is to revert #432 (and reopen bug #396) until a more robust solution can be found.

@cgspeck
Copy link
Contributor

cgspeck commented Oct 28, 2019

I was wondering if the root cause has been identified or if is any fix for this problem?

I'm also getting the same error when I attempt to delete a fermentable from a recipe, in the console it says:

[21:14:49.788] ERROR : void Database::removeIngredientFromRecipe(Recipe*, BeerXMLElement*) failed to delete ingredient. DELETE FROM fermentable where id=597 FOREIGN KEY constraint failed Unable to fetch row
Qt has caught an exception thrown from an event handler. Throwing
exceptions from an event handler is not supported in Qt.
You must not let any exception whatsoever propagate through Qt code.
If that is not possible, in Qt 5 you must at least reimplement
QCoreApplication::notify() and catch all exceptions there.

I was able to manually clean up my database by running the following script:

DELETE FROM fermentable_children WHERE child_id = 597;
DELETE FROM fermentable_in_recipe WHERE fermentable_id = 597;
DELETE FROM fermentable_in_inventory WHERE fermentable_id = 597;
DELETE FROM fermentable where id=597;

@mikfire
Copy link
Contributor

mikfire commented Oct 28, 2019

I'll look. I don't think I've seen that one but I also can't claim to have been looking.

@cgspeck
Copy link
Contributor

cgspeck commented Nov 4, 2019

This problem has become severe for me. It triggers every time I attempt to remove a fermentable from a recipe.

Is there anything I can do to help diagnose/fix this issue?

Is there any way I can clean or verify the integrity of my database?

@cgspeck
Copy link
Contributor

cgspeck commented Nov 5, 2019

I had a look into this and made a change to database.cpp which essentially runs the SQL I mentioned above and made the application usable for me once more.

I have no idea how this actually interacts with the inventory system. If someone can review it and approve or give feedback that would be great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants