Skip to content
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

[ActiveRecord] Superclass for aborted queries #36694

Merged
merged 1 commit into from Jul 16, 2019

Conversation

@kirs
Copy link
Member

commented Jul 16, 2019

These days, when developer wants to rescue all possible timeouts (but not other query exceptions), they have to do rescue ActiveRecord::QueryCancelled, ActiveRecord::StatementTimeout, ActiveRecord::AdapterTimeout. It's too easy to miss one and don't rescue what you expected.

I'd like to introduce a single super class for all timeout errors to make it easy for developers to do rescue ActiveRecord::QueryAborted.

cc @casperisfine @rafaelfranca @Edouard-chin @bryanparadis

@rails-bot rails-bot bot added the activerecord label Jul 16, 2019

@@ -353,20 +353,24 @@ class Deadlocked < TransactionRollbackError
class IrreversibleOrderError < ActiveRecordError
end

# Superclass for all timeout-related errors
class Timeout < StatementInvalid

This comment has been minimized.

Copy link
@Edouard-chin

Edouard-chin Jul 16, 2019

Contributor

Does it make sense that Timeout still inherits from StatementInvalid? It was previously like this for backward compatibility reasons , but does it still apply now ? cc/ @byroot

This comment has been minimized.

Copy link
@rafaelfranca

rafaelfranca Jul 16, 2019

Member

Yes. We still need to keep it backward compatible.

@kirs kirs force-pushed the kirs:timeout-error-superclass branch from 71fd438 to 9866497 Jul 16, 2019

@kirs kirs force-pushed the kirs:timeout-error-superclass branch from 9866497 to 730d810 Jul 16, 2019

@kirs kirs changed the title [ActiveRecord] Superclass for timeout errors [ActiveRecord] Superclass for aborted queries Jul 16, 2019

@kirs

This comment has been minimized.

Copy link
Member Author

commented Jul 16, 2019

@rafaelfranca thanks for feedback, updated!

@rafaelfranca rafaelfranca merged commit 1b984fc into rails:master Jul 16, 2019

2 checks passed

buildkite/rails Build #62270 passed (9 minutes, 7 seconds)
Details
codeclimate All good!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.