Improve details output

Changes

@@ -17,6 +17,7 @@ Revision history for Perl extension App::Sqitch
      - Removed a wayward mention of the long-deprecated `SQITCH_URI`
        environment variable from the Oracle tutorial. Thanks to Austin Hanson
        for the report (#845).
+     - Improved error output by including any previous exception.
 
 1.4.1  2024-02-04T16:35:32Z
      - Removed the quoting of the role and warehouse identifiers that was

lib/App/Sqitch.pm

@@ -213,13 +213,13 @@ sub go {
         if ($_->exitval == 1) {
             # Non-fatal exception; just send the message to info.
             $sqitch->info($_->message);
+        } elsif ($_->ident eq 'DEV') {
+            # Vent compmlete detaiosl of fatal DEV error.
+            $sqitch->vent($_->as_string);
         } else {
-            # Fatal exception; vent.
+            # Vent fatal error message, trace details.
             $sqitch->vent($_->message);
-
-            # Emit the stack trace. DEV errors should be vented; otherwise trace.
-            my $meth = $_->ident eq 'DEV' ? 'vent' : 'trace';
-            $sqitch->$meth($_->stack_trace->as_string);
+            $sqitch->trace($_->details_string);
         }
 
         # Bail.

lib/App/Sqitch/X.pm

@@ -54,13 +54,21 @@ sub hurl {
 
 sub as_string {
     my $self = shift;
-    join "\n", grep { defined } (
+    join "\n", grep { $_ } (
         $self->message,
         $self->previous_exception,
         $self->stack_trace
     );
 }
 
+sub details_string {
+    my $self = shift;
+    join "\n", grep { $_ } (
+        $self->previous_exception,
+        $self->stack_trace
+    );
+}
+
 1;
 
 __END__
@@ -148,6 +156,13 @@ used for string overloading of the exception object, which means it is the
 output shown for uncaught exceptions. Its contents are the concatenation of
 the exception message, the previous exception (if any), and the stack trace.
 
+=head3 C<details_string>
+
+  my $errstr = $x->details;
+
+Returns the the stringified version of the previous exception (if any), and
+the stack trace.
+
 =head1 Handling Exceptions
 
 use L<Try::Tiny> to do exception handling, like so:

t/base.t

@@ -2,7 +2,7 @@
 
 use strict;
 use warnings;
-use Test::More tests => 214;
+use Test::More tests => 224;
 # use Test::More 'no_plan';
 use Test::MockModule 0.17;
 use Path::Class;
@@ -138,8 +138,8 @@ GO: {
     my $sqitch_mock = Test::MockModule->new($CLASS);
     my @vented;
     $sqitch_mock->mock(vent => sub { shift; push @vented => @_ });
-    my $traced;
-    $sqitch_mock->mock(trace => sub { $traced = $_[1]; });
+    my @traced;
+    $sqitch_mock->mock(trace => sub { shift; push @traced => @_ });
     my @infoed;
     $sqitch_mock->mock(info => sub { shift; push @infoed => @_ });
     my @emitted;
@@ -153,29 +153,47 @@ GO: {
     is_deeply \@vented, ['OMGWTF!'], 'The error should have been vented';
     is_deeply \@infoed, [], 'Should have no info output';
     is_deeply \@emitted, [], 'Should have no emitted output';
-    is $traced, $ex->stack_trace->as_string,
+    is_deeply \@traced, [$ex->stack_trace->as_string],
         'The stack trace should have been sent to trace';
 
+    # Make it die with a previous exception.
+    $ex = puke(ident => 'yikes', message => 'Yikes!', previous_exception => 'Invalid snicker');
+    @vented = @traced = ();
+    is $sqitch->go, 2, 'Go should return 2 on next Sqitch exception';
+    is_deeply \@vented, ['Yikes!'], 'The next error should have been vented';
+    is_deeply \@infoed, [], 'Should have no info output';
+    is_deeply \@emitted, [], 'Should have no emitted output';
+    is_deeply \@traced, ["Invalid snicker\n" . $ex->stack_trace->as_string],
+        'The previous exceptin and stack trace should have been sent to trace';
+
     # Make it die with a developer exception.
-    @vented = ();
-    $traced = undef;
+    @vented = @traced = ();
     $ex = puke( message => 'OUCH!', exitval => 4 );
     is $sqitch->go, 4, 'Go should return exitval on another exception';
-    is_deeply \@vented, ['OUCH!', $ex->stack_trace->as_string],
+    is_deeply \@vented, ["OUCH!\n" . $ex->stack_trace->as_string],
         'Both the message and the trace should have been vented';
     is_deeply \@infoed, [], 'Should still have no info output';
     is_deeply \@emitted, [], 'Should still have no emitted output';
-    is $traced, undef, 'Nothing should have been traced';
+    is_deeply \@traced, [], 'Nothing should have been traced';
+
+    # Make it die with a developer previous exception.
+    @vented = @traced = ();
+    $ex = puke( message => 'OOOF!', exitval => 3, previous_exception => 'Cannot open file' );
+    is $sqitch->go, 3, 'Go should return exitval on wrapped exception';
+    is_deeply \@vented, ["OOOF!\nCannot open file\n" . $ex->stack_trace->as_string],
+        'Should have vented the message, previous exception, and trace';
+    is_deeply \@infoed, [], 'Should still have no info output';
+    is_deeply \@emitted, [], 'Should still have no emitted output';
+    is_deeply \@traced, [], 'Nothing should have been traced';
 
     # Make it die with a non-fatal exception (error code 1)
     @vented = ();
-    $traced = undef;
     $ex = puke( message => 'OOPS!', exitval => 1 );
     is $sqitch->go, 1, 'Go should return exitval on non-fatal exception';
     is_deeply \@vented, [], 'Should not have vented';
     is_deeply \@infoed, ['OOPS!'], 'Should have sent the message to message';
     is_deeply \@emitted, [], 'Should still have no emitted output';
-    is $traced, undef, 'Nothing should have been traced';
+    is_deeply \@traced, [], 'Nothing should have been traced';
 
     # Make it die without an exception object.
     $ex = 'LOLZ';
@@ -185,7 +203,7 @@ GO: {
     like $vented[0], qr/^LOLZ\b/, 'And it should include our message';
     is_deeply \@infoed, [], 'Should again have no info output';
     is_deeply \@emitted, [], 'Should still have no emitted output';
-    is $traced, undef, 'Nothing should have been traced';
+    is_deeply \@traced, [], 'Nothing should have been traced';
 }
 
 ##############################################################################

t/x.t

@@ -45,19 +45,34 @@ is $x->message, 'OMFG!', 'The message should have been passed';
 is $x->exitval, 2, 'Exit val should again be 2';
 is $x->previous_exception, 'Yo dawg',
     'Previous exception should have been passed';
+is $x->as_string, join("\n",
+    $x->message,
+    $x->previous_exception,
+    $x->stack_trace
+), 'Stringification should work';
+
+is $x->as_string, "$x", 'Stringification should work';
+
+is $x->details_string, join("\n",
+    $x->previous_exception,
+    $x->stack_trace
+), 'Details string should work';
 
 throws_ok { hurl {ident => 'blah', message => 'OMFG!', exitval => 1} } $CLASS;
 isa_ok $x = $@, $CLASS, 'Thrown object';
 is $x->message, 'OMFG!', 'The params should have been passed';
 is $x->exitval, 1, 'Exit val should be 1';
-is $x->as_string, join("\n", grep { defined }
+is $x->as_string, join("\n",
     $x->message,
-    $x->previous_exception,
     $x->stack_trace
 ), 'Stringification should work';
 
 is $x->as_string, "$x", 'Stringification should work';
 
+is $x->details_string, join("\n",
+    $x->stack_trace
+), 'Details string should work';
+
 # Do some actual exception handling.
 try {
     hurl io => 'Cannot open file';