-
Notifications
You must be signed in to change notification settings - Fork 40
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
Investigate more robust error-handling for database transactions in storage.py #1440
Comments
More evidence in support of this inquiry: I experienced the following crash after (1) using the Journalist Interface to delete all but 3 of ~200 source accounts previously synced and then (2) reopening the Client, which got only partway through its initial sync:
In other words, two Client sessions were necessary to catch up to a bulk operation performed in the Journalist Interface. |
In the course of #1634, I would zoom out from this goal and suggest instead that what we need is a consistent way to mutate state across the application, which is a separate concern from how that state is persisted (and refreshed). As it is, I believe we're effectively letting transactionality introduce data races within the running application. #1634 will offer some baby steps for how we might think and go about this. |
Briefly, here are my findings from #1634 → #1661:
I have snippets for what these patterns could look like in the Client; they're just not workable at the level of individual models as in #1634 and #1661. |
Description
In working on eg #1432, I've had the chance to get more familiar with
storage.py
, and it might make sense to investigate improvements we could make to our database transaction handling.For example: a complex method like this one where we have a single
commit()
at the end might benefit from savepoints and some opportunities to roll back an invalid transaction if one is encountered (and some agreements about what that behaviour should be). We might want to standardize on one procedure that we use whenever doing database transactions. As a later goal, we might want to wrap database errors in our own exceptions instead of handling them in GUI classes.This could especially be relevant for future features like multi-select or multi-delete, or other scenarios where we could increase the likelihood of encountering database errors. I'm just filing this issue so that we can set aside a little time for this, and also, to discuss what we'd like the behaviour to be.
How will this impact SecureDrop users?
Makes Workstation more robust, more suitable for heavier use (large numbers of sources/transactions, multiple Workstation computers per server instance, etc)
How would this affect the SecureDrop Workstation threat model?
No major changes; potential benefits to stability and/or reliability of product
User Stories
As a Workstation user, I want a smooth operating experience, including clean error-handling and predictable behaviour with large numbers of database transactions
The text was updated successfully, but these errors were encountered: