fix: fix RPC behavior when exception is thrown across boundary

.gitignore

@@ -15,6 +15,7 @@ nytprof.out
 
 # Dizt::Zilla
 /.build/
+/deps/
 
 # Module::Build
 _build/
@@ -33,6 +34,7 @@ inc/
 /MANIFEST.bak
 /pm_to_blib
 /*.zip
+/Myriad-*/
 
 # IDEs ( Some people !)
 .idea

lib/Myriad/Exception/Base.pm

@@ -27,7 +27,15 @@ use overload '""' => sub { shift->as_string }, bool => sub { 1 }, fallback => 1;
 
 sub new {
     my ($class, %args) = @_;
-    bless \%args, $class
+    # Force the reason to be a valid string
+    # or it would cause issue with strongly typed clients.
+    # Moreover, attempt to use blessed object here would crash the listener
+    if ($args{reason}) {
+        my $reason = "$args{reason}";
+        $reason =~ s/(\s+)$//;
+        $args{reason} = $reason;
+    }
+    bless \%args, $class;
 }
 
 =head2 reason

lib/Myriad/RPC.pm

@@ -81,6 +81,16 @@ declare_exception UnknownTransport => (
     message => 'Unknown transport'
 );
 
+=head2 RemoteException
+
+An exception has been thrown from remote service
+
+=cut
+
+declare_exception RemoteException => (
+    message => 'The RPC method failed at server side'
+);
+
 =head1 METHODS
 
 =cut

lib/Myriad/RPC/Client/Implementation/Memory.pm

@@ -88,14 +88,20 @@ async method call_rpc ($service, $method, %args) {
             $self->loop->timeout_future(at => $deadline),
             $pending
         );
+
+        unless (exists $message->response->{response}) {
+            my $reason = $message->response->{error}{message} // "Unknown";
+            Myriad::Exception::RPC::RemoteException->throw(reason => "Remote exception is thrown: $reason");
+        }
+
         return $message->response->{response};
     } catch ($e) {
         if ($e =~ /Timeout/) {
             $e  = Myriad::Exception::RPC::Timeout->new(reason => 'deadline is due');
         } else {
             $e = Myriad::Exception::InternalError->new(reason => $e) unless blessed $e && $e->DOES('Myriad::Exception');
         }
-        $pending->fail($e);
+        $pending->fail($e) unless $pending->is_ready;
         delete $pending_requests->{$message_id};
         $e->throw();
     }

lib/Myriad/RPC/Client/Implementation/Redis.pm

@@ -97,6 +97,11 @@ async method call_rpc($service, $method, %args) {
             $pending
         );
 
+        unless (exists $message->response->{response}) {
+            my $reason = $message->response->{error}{message} // "Unknown";
+            Myriad::Exception::RPC::RemoteException->throw(reason => "Remote exception is thrown: $reason");
+        }
+
         return $message->response->{response};
     } catch ($e) {
         $log->warnf('Failed on RPC call - %s', $e);
@@ -105,7 +110,7 @@ async method call_rpc($service, $method, %args) {
         } else {
             $e = Myriad::Exception::InternalError->new(reason => $e) unless blessed $e && $e->DOES('Myriad::Exception');
         }
-        $pending->fail($e);
+        $pending->fail($e) unless $pending->is_ready;
         delete $pending_requests->{$message_id};
         $e->throw();
     }

t/RPC/exceptions.t

@@ -0,0 +1,82 @@
+use strict;
+use warnings;
+
+use Test::More;
+use Test::Deep;
+use Test::Fatal;
+use Test::Myriad;
+
+use Future;
+use Future::AsyncAwait;
+use Object::Pad qw(:experimental);
+use Myriad::Exception::InternalError;
+
+my ($broken_services);
+
+# Example of some broken behavior that implementations can have
+package Test::BrokenServices {
+    use Myriad::Service;
+    async method normal : RPC (%args) {
+        return {pong => 1};
+    }
+    async method normal_dead : RPC (%args) {
+        die "I am died";
+    }
+    async method hash_dead : RPC (%args) {
+        die {reason => "I am died"};
+    }
+    async method blessed_dead : RPC (%args) {
+        die bless {reason => "I am died"}, "Test::Exceptions";
+    }
+    async method bubbled_dead : RPC (%args) {
+        # Fake it as something is wrong with Myriad itself
+        Myriad::Exception::InternalError->throw();
+    }
+}
+
+BEGIN {
+    $broken_services = Test::Myriad->add_service(service => 'Test::BrokenServices');
+}
+
+
+await Test::Myriad->ready();
+
+subtest 'Normal RPC calls must succeeded as usual' => sub {
+    my $resposne = $broken_services->call_rpc('normal')->get;
+    cmp_deeply($resposne, {pong => 1});
+};
+
+subtest 'Dead RPC calls must have its exceptions bubbled up to caller' => sub {
+    my $failure = exception {
+        $broken_services->call_rpc('normal_dead')->get;
+    };
+    is(ref $failure, "Myriad::Exception::RPC::RemoteException", "Correct error received by caller");
+    like($failure->reason, qr/reason=I am died/, "Correct error message");
+};
+
+subtest 'HASHREFs must not cross RPC boundary!' => sub {
+    my $failure = exception {
+        $broken_services->call_rpc('hash_dead')->get;
+    };
+    is(ref $failure, "Myriad::Exception::RPC::RemoteException", "Correct error received by caller");
+    like($failure->reason, qr/reason=HASH\(0x[a-zA-Z0-9]+\)/, "Correct error message");
+};
+
+subtest 'blessed HASHREFs must not cross RPC boundary!' => sub {
+    my $failure = exception {
+        $broken_services->call_rpc('blessed_dead')->get;
+    };
+    is(ref $failure, "Myriad::Exception::RPC::RemoteException", "Correct error received by caller");
+    like($failure->reason, qr/reason=Test::Exceptions=HASH\(0x[a-zA-Z0-9]+\)/, "Correct error message");
+};
+
+subtest 'Internal Myriad errors on remote must not appear as local Myriad error!' => sub {
+    my $failure = exception {
+        $broken_services->call_rpc('bubbled_dead')->get;
+    };
+    is(ref $failure, "Myriad::Exception::RPC::RemoteException", "Correct error received by caller");
+    is($failure->reason, 'Remote exception is thrown: Internal error (category=internal)', "Correct error message");
+};
+
+done_testing();
+

t/exception.t

@@ -18,6 +18,7 @@ subtest 'can declare exceptions' => sub {
     is($ex->message, 'this is a message (category=some_category)', 'message is correct');
     is($ex->category, 'some_category', 'category is correct');
     is("$ex", 'this is a message (category=some_category)', 'stringifies too');
+    ok($ex->DOES('Myriad::Exception'), 'It inherits from Myriad::Exception role');
 };
 
 done_testing;