element-hq / element-hq/synapse
`StopIteration` is silently ignored inside database transactions
- Dominant language
- Python
- Stars
- 4.6k
- Forks
- 600
- Avg merge
- 5d 22h
- Merged PRs (30d)
- 51
Description
*Spawning from https://github.com/element-hq/synapse/pull/17512#discussion_r1715954505,*
If you accidentally cause a `StopIteration` exception to be thrown inside a transaction (`txn` stuff), nothing will appear in the logs even though everything blew up. It never reaches our [`try`/`except`](https://github.com/element-hq/synapse/blob/a308d99f30d7e660115e355c54c37ac149cdbe53/synapse/storage/database.py#L938-L941) block that normally handles logging the exceptions.
You can easily run into this by using something like `next(iter(my_map.keys()))` (to grab the first key in the iterable) where `my_map` is just an empty dictionary. But this problem is easily reproduced by just doing `raise StopIteration` directly.
### Dev notes
Seems related to https://peps.python.org/pep-0479/ but that makes it seem like with that change ([since Python 3.7](https://docs.python.org/3/whatsnew/3.7.html#changes-in-python-behavior)), `StopIteration` shouldn't be silently ignored like it is now.
I'm guessing this is a bad interaction between the `with` context managers at various points and `make_deferred_yieldable(...)` swallowing it.
There have been more recent related issues around this like https://github.com/python/cpython/issues/112182
Contributor guide
Research direction
Start in synapse/storage/database.py around the try/except at lines 938-941 and reproduce the failure with a direct StopIteration inside a transaction. Trace the surrounding context managers and make_deferred_yieldable(...) to find where the exception is swallowed; done means the failure is no longer silent and transaction exception logging still works.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100