[openstreetmap/openstreetmap-website] Api timeout rollbacks (#1381)
notifications at github.com
Mon Nov 28 11:58:35 UTC 2016
This seems like it would be the default behaviour, but because the exception that [Timeout::timeout raises on the other thread](https://github.com/ruby/ruby/blob/v2_3_1/lib/timeout.rb#L106) [doesn't appear to be rescue-able](https://gist.github.com/zerebubuth/8211e07e9f592e5b8146dea2344906b7), [Rails doesn't detect it](https://github.com/rails/rails/blob/v22.214.171.124/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb#L185). However, it still runs the ensure block, which [attempts to commit](https://github.com/rails/rails/blob/v126.96.36.199/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb#L193-L198) the transaction.
The result of this is that a long-running transaction can potentially be interrupted while partially complete and will commit the partial result rather than rolling back.
I've tried to fix this by using `Timeout.timeout`'s second argument, which allows a custom exception to be thrown. On the API side, we were doing this anyway, so hopefully the behaviour is unchanged. On the web side, I've added in some extra logic to treat the API error as if it were a `Timeout::Error`.
Thanks to @Zverik for pointing this out.
You can view, comment on, or merge this pull request online at:
-- Commit Summary --
* Make API and web roll back any open transactions on timeout.
* Try to cover all code paths.
-- File Changes --
M app/controllers/application_controller.rb (10)
-- Patch Links --
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the rails-dev