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

AR Trilogy Adapter: Handle AdapterTimeout and ConnectionFailed errs #497

Merged
merged 2 commits into from
Jun 1, 2023

Conversation

adrianna-chang-shopify
Copy link
Contributor

@adrianna-chang-shopify adrianna-chang-shopify commented May 31, 2023

See #497

This PR ensures that ActiveRecord::AdapterTimeout errors are handled for the AR Trilogy Adapter. As of rails/rails@785f656, TimeoutErrors are translated to AdapterTimeout errors in Rails. As such, we no longer need special handling in #acquire_semian_resource for such errors.

This commit also ensures that ActiveRecord::ConnectionFailed errors are classified as resource exceptions. These are network-related exceptions and should be handled by Semian. I've added a corresponding test.

cc @casperisfine @etiennebarrie

lib/semian/activerecord_trilogy_adapter.rb Outdated Show resolved Hide resolved
This commit ensures that ActiveRecord::AdapterTimeout errors are
handled for the AR Trilogy Adapter. As of rails/rails@785f656,
TimeoutErrors are translated to AdapterTimeout errors in Rails. As such,
we no longer need special handling in `#acquire_semian_resource` for
such errors.

This commit also ensures that ActiveRecord::ConnectionFailed errors are
classified as resource exceptions. These are network-related exceptions
and should be handled by Semian.
@adrianna-chang-shopify adrianna-chang-shopify merged commit d6ba307 into master Jun 1, 2023
43 checks passed
@adrianna-chang-shopify adrianna-chang-shopify deleted the ac-resource-exceptions branch June 1, 2023 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants