`destroy` shouldn't raise when child associations fail to save

Deep down in the association internals, we're calling `destroy!` rather
than `destroy` when handling things like `dependent` or autosave
association callbacks. Unfortunately, due to the structure of the code
(e.g. it uses callbacks for everything), it's nearly impossible to pass
whether to call `destroy` or `destroy!` down to where we actually need
it.

As such, we have to do some legwork to handle this. Since the callbacks
are what actually raise the exception, we need to rescue it in
`ActiveRecord::Callbacks`, rather than `ActiveRecord::Persistence` where
it matters. (As an aside, if this code wasn't so callback heavy, it
would handling this would likely be as simple as changing `destroy` to
call `destroy!` instead of the other way around).

Since we don't want to lose the exception when `destroy!` is called (in
particular, we don't want the value of the `record` field to change to
the parent class), we have to do some additional legwork to hold onto it
where we can use it.

Again, all of this is ugly and there is definitely a better way to do
this. However, barring a much more significant re-architecting for what
I consider to be a reletively minor improvement, I'm willing to take
this small hit to the flow of this code (begrudgingly).

1 files changed, 3 insertions, 0 deletions

diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb
index 19f0dca5a6..c7c769b283 100644
--- a/activerecord/lib/active_record/callbacks.rb
+++ b/activerecord/lib/active_record/callbacks.rb
@@ -290,6 +290,9 @@ module ActiveRecord
 
     def destroy #:nodoc:
       _run_destroy_callbacks { super }
+    rescue RecordNotDestroyed => e
+      @_association_destroy_exception = e
+      false
     end
 
     def touch(*) #:nodoc: