Fix crash on adjustment error #4214
Conversation
|
@dorner Noting that rubocop is unhappy with this atm. |
|
(nods) The one weakness in the event sourcing stuff is that if something goes bad, it is going to basically prevent further entry. On the other hand, that does mean we are going to get informed about issues that do occur fairly quickly! |
|
@dorner We do need to have a discussion on how we troubleshoot/fix things if we do get an error like this -- right now we have a "Beer Truck Problem". |
| rescue Errors::InsufficientAllotment, InventoryError => e | ||
| @adjustment.errors.add(:base, e.message) | ||
| raise ActiveRecord::Rollback |
There was a problem hiding this comment.
quick question just out of curiosity. in the PR you write
When the error was raised, it wouldn't roll back the transaction - so it would still do the adjustment. Again, this has been around for a long time. :(
Raising any error vs an ActiveRecord::Rollback wouldn't affect rollbacks happening? Like regardless of what error is raised the rollback should happen, right?
I'm just curious if the changes do address that issue, or if that is not longer an issue, cause I don't really see why either version would have an issue with rolling back.
There was a problem hiding this comment.
The issue is that the error was being caught, and a value was added to the errors of the record. But we then just raised the error again, and at that point (I think?) it was already outside of the transaction, so a rollback didn't happen. I went back and forth a lot with this PR - maybe the issue with the rollback was in one of the intermediate states.
|
Pushed lint fix. |
|
@dorner: Your PR |
There were 4 separate issues here:
The original error seemed to have been caused by issues with re-running events (meaning there was a bad event that got in somehow which indicates that any further actions would all fail).