Skip to content

Commit e609aac

Browse files
committed
fix(EventHandler): require handlers to return with a clean SQLAlchemy session
In processing a batch with multiple events, if a handler returns with uncommitted changes, the next handler to commit will include them. process() now requires each handler to have either successfully committed or rolled back all of its own changes. In other words, each event now runs in its own transaction.
1 parent b868684 commit e609aac

1 file changed

Lines changed: 11 additions & 1 deletion

File tree

securedrop/journalist_app/api2/events.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,17 @@ class EventHandler:
3838
`journalist_api2.types.EVENT_DATA_TYPES`;
3939
4040
3. define the handler as a static method `handle_thing_done(event: Event)`
41-
in this class
41+
in this class; and
4242
4343
4. explicitly register `{"thing_done": self.handle_thing_done}` inside
4444
`EventHandler.process()`.
4545
4646
This is belt-and-suspenders for ensuring that only the intended methods are
4747
exposed as callable event handlers.
48+
49+
To preserve transaction separation between events, handlers MUST return with
50+
a clean SQLAlchemy session: in other words, having either successfully
51+
committed or rolled back all of their changes.
4852
"""
4953

5054
def __init__(self, session: Session, redis: Redis) -> None:
@@ -89,9 +93,15 @@ def process(self, event: Event, minor: int) -> EventResult:
8993
try:
9094
result = handler(event, minor)
9195

96+
# Enforce "handlers MUST return with a clean SQLAlchemy session" above:
97+
assert not db.session.dirty # noqa: S101
98+
assert not db.session.new # noqa: S101
99+
assert not db.session.deleted # noqa: S101
100+
92101
# Catch anything not handled by the handler:
93102
except Exception:
94103
current_app.logger.error(f"unhandled exception in handler for {event}", exc_info=True)
104+
db.session.rollback()
95105
result = EventResult(
96106
event.id, (EventStatusCode.InternalServerError, "failed to process event")
97107
)

0 commit comments

Comments
 (0)