From: Peter Rabbitson <ribasushi@cpan.org>
Date: Mon, 10 Aug 2015 15:29:05 +0000 (+0200)
Subject: Add is_exception check to the result of exception_action (inspired by GHPR#15)
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=e240b8ba6a26e17bed8e87235bcc201eefca350d;p=dbsrgits%2FDBIx-Class-Historic.git

Add is_exception check to the result of exception_action (inspired by GHPR#15)

This builds upon work in 841efcb3 and 9bea2000, tying loose ends before next
commits

Read under -w
---

diff --git a/lib/DBIx/Class/Schema.pm b/lib/DBIx/Class/Schema.pm
index 7b5ae64..36041bd 100644
--- a/lib/DBIx/Class/Schema.pm
+++ b/lib/DBIx/Class/Schema.pm
@@ -8,7 +8,7 @@ use base 'DBIx::Class';
 use DBIx::Class::Carp;
 use Try::Tiny;
 use Scalar::Util qw/weaken blessed/;
-use DBIx::Class::_Util qw(refcount quote_sub);
+use DBIx::Class::_Util qw(refcount quote_sub is_exception);
 use Devel::GlobalDestruction;
 use namespace::clean;
 
@@ -1055,26 +1055,37 @@ default behavior will provide a detailed stack trace.
 =cut
 
 sub throw_exception {
-  my $self = shift;
+  my ($self, @args) = @_;
 
   if (my $act = $self->exception_action) {
-    if ($act->(@_)) {
-      DBIx::Class::Exception->throw(
+    try {
+      # if it throws - good, we'll go down to the catch
+      # if it doesn't - do different things depending on RV truthiness
+      if( $act->(@args) ) {
+        $args[0] = (
           "Invocation of the exception_action handler installed on $self did *not*"
         .' result in an exception. DBIx::Class is unable to function without a reliable'
         .' exception mechanism, ensure that exception_action does not hide exceptions'
-        ." (original error: $_[0])"
-      );
-    }
+        ." (original error: $args[0])"
+        );
+      }
+      else {
+        carp_unique (
+          "The exception_action handler installed on $self returned false instead"
+        .' of throwing an exception. This behavior has been deprecated, adjust your'
+        .' handler to always rethrow the supplied error.'
+        );
+      }
+    } catch {
+      # We call this to get the necessary warnings emitted and disregard the RV
+      # as it's definitely an exception if we got as far as catch{}
+      is_exception($_);
 
-    carp_unique (
-      "The exception_action handler installed on $self returned false instead"
-    .' of throwing an exception. This behavior has been deprecated, adjust your'
-    .' handler to always rethrow the supplied error.'
-    );
+      die $_;
+    };
   }
 
-  DBIx::Class::Exception->throw($_[0], $self->stacktrace);
+  DBIx::Class::Exception->throw( $args[0], $self->stacktrace );
 }
 
 =head2 deploy
diff --git a/t/storage/txn.t b/t/storage/txn.t
index f8a729f..ea9845f 100644
--- a/t/storage/txn.t
+++ b/t/storage/txn.t
@@ -453,6 +453,17 @@ warnings_are {
   } qr/\QTransaction aborted: $broken_exception. Rollback failed: lost connection to storage at @{[__FILE__]} line $ln\E\n/;  # FIXME wtf - ...\E$/m doesn't work here
 
   is @w, 1, 'One matching warning only';
+
+  # try the same broken exception object, but have exception_action inject it
+  $s->exception_action(sub { die $broken_exception });
+  eval {
+    $s->txn_do( sub {
+      die "some string masked away";
+    });
+  };
+  isa_ok $@, 'DBICTest::BrokenOverload', 'Deficient exception properly propagated';
+
+  is @w, 2, 'The warning was emitted a second time';
 }
 
 done_testing;