Skip to content

Commit

Permalink
address reviews
Browse files Browse the repository at this point in the history
  • Loading branch information
ngeojiajun-deriv committed Feb 22, 2024
1 parent c4ff571 commit 132b176
Show file tree
Hide file tree
Showing 5 changed files with 3 additions and 28 deletions.
22 changes: 0 additions & 22 deletions lib/Myriad/Exception/Base.pm
Original file line number Diff line number Diff line change
Expand Up @@ -46,28 +46,6 @@ Returns the exception message as a string.

sub as_string { shift->message }

=head2 does
Check the role ownership of the objects, added here to maintain compatibility with Object::Pad exports.
This is needed as some part of Myriad code assume that the objects created by Myriad exceptions do exports does,
which in turn to be absent.
=cut

sub does {
use Object::Pad::MOP::Class qw(:experimental);
my ($self, $role) = @_;

return 0 unless $role and ref $self;

my $klass = ref $self;
my $klass_ptr = Object::Pad::MOP::Class->try_for_class($klass);
return 0 unless defined $klass_ptr;

my %roles = map {($_->name) => 1} $klass_ptr->all_roles;
return $roles{$role} // 0;
}

1;

=head1 AUTHOR
Expand Down
2 changes: 1 addition & 1 deletion lib/Myriad/RPC/Client/Implementation/Memory.pm
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ async method call_rpc ($service, $method, %args) {
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');
$e = Myriad::Exception::InternalError->new(reason => $e) unless (ref $e) =~ /^Myriad::Exception::/;
}
$pending->fail($e) unless $pending->is_ready;
delete $pending_requests->{$message_id};
Expand Down
2 changes: 1 addition & 1 deletion lib/Myriad/RPC/Client/Implementation/Redis.pm
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ async method call_rpc($service, $method, %args) {
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');
$e = Myriad::Exception::InternalError->new(reason => $e) unless (ref $e) =~ /^Myriad::Exception::/;
}
$pending->fail($e) unless $pending->is_ready;
delete $pending_requests->{$message_id};
Expand Down
2 changes: 1 addition & 1 deletion lib/Myriad/Service/Implementation.pm
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ async method load () {
} catch ($e) {
# Pass through the myriad exception, use indirect exports from Myriad::Exception::Base
# for simplicity
unless (blessed $e and Myriad::Exception::Base::does($e, 'Myriad::Exception')) {
unless ((ref $e) =~ /^Myriad::Exception::/) {
# Ensure that we are passing the exception reason only by means of scalar
# or it would cause issue with strongly typed clients.
# Moreover, attempt to use blessed object would crash the listener
Expand Down
3 changes: 0 additions & 3 deletions t/exception.t
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ 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');
# Some of the Myriad RPC code expect the does method is present but it doesn't in old version
# This would enforce it
ok($ex->does('Myriad::Exception'), 'It inherits from Myriad::Exception role');
};

done_testing;
Expand Down

0 comments on commit 132b176

Please sign in to comment.