Add option to stop swallowing errors on callbacks. #16537
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -375,6 +363,21 @@ def restore_transaction_record_state(force = false) #:nodoc: | |||
| end | |||
| end | |||
|
|
|||
| protected | |||
There was a problem hiding this comment.
this code didnt change, just had to move it, so I can have access to clear_transaction_record_state force_clear_transaction_record_state restore_transaction_record_state
|
@chancancode I guess I addressed all your concerns:
|
| @@ -3,11 +3,21 @@ module ActiveRecord | |||
| module Transactions | |||
| extend ActiveSupport::Concern | |||
| ACTIONS = [:create, :destroy, :update] | |||
| CALLBACK_WARN_MESSAGE = <<-EOF | |||
| Currently `config.active_record.raise_in_transactional_callbacks` is set to false, | |||
There was a problem hiding this comment.
I'll start with describing the behavior (i.e. errors are only logged) and the future change. The "option" is really just an opt-in flag and should be mentioned last. (Same for the changelog entry)
But we can also improve this after merging.
Currently, Active Record will rescue any errors raised within
after_rollback/after_create callbacks and print them to the
logs. Next versions of rails will not rescue those errors anymore,
and just bubble them up, as the other callbacks.
This adds a opt-in flag to enable that behaviour, of not rescuing
the errors.
Example:
# For not swallow errors in after_commit/after_rollback
config.active_record.errors_in_transactional_callbacks = true
[fixes rails#13460]
|
@chancancode Warning message, CHANGELOG, and commit message updated. |
chancancode
added a commit
that referenced
this pull request
Aug 18, 2014
Add option to stop swallowing errors on callbacks.
| @@ -63,13 +67,19 @@ def rollback | |||
| end | |||
|
|
|||
| def rollback_records | |||
| records.uniq.each do |record| | |||
| ite = records.uniq | |||
There was a problem hiding this comment.
Is ite an acronym for something?
|
Perhaps new apps should be generated with the config option already set? |
|
@jonleighton yep, it's been added in c7c2df2 (Thanks for taking your time to help review this by the way!) |
pjambet
added a commit
to harrystech/rails-pipeline
that referenced
this pull request
Oct 31, 2014
By default Rails swallows exception in after_{commit/rollback} hooks,
therefore prevent specs to fail.
By moving the callbacks inside the transaction in test environment any
exceptions thrown will correctly be bubbled up.
There is an option to fix that in rails 4.2
rails/rails#16537
faun
added a commit
to zinedistro/zinedistro
that referenced
this pull request
Jan 29, 2015
See [#14488] [1] and [#16537] [2] for more details. [1]: [rails/rails#14488] [2]: [rails/rails#16537]
faun
added a commit
to zinedistro/zinedistro
that referenced
this pull request
Jan 29, 2015
See [#14488] and [#16537] for more details. [#14488]: [rails/rails#14488] [#16537]: [rails/rails#16537]
faun
added a commit
to zinedistro/zinedistro
that referenced
this pull request
Jan 29, 2015
See [#14488] and [#16537] for more details. [#14488]: [rails/rails#14488] [#16537]: [rails/rails#16537]
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Seconds take of #14488 .
After the transaction refactor, we can cleanly apply this patch, as we have a better control of the stack of transactions, and we know that even when raising errors, the transaction wont leaky into the stack.
Description
Add option to stop swallowing standard errors on
after_commitandafter_rollback.Currently the option will be +false+ and throw a warning untill set it to +true+.
Example:
The option should be removed in future versions of Rails, as the desired
behaviour is not to swallow exceptions.
[fixes #13460]
review @jonleighton @rafaelfranca @tenderlove @chancancode @senny