[ 
https://issues.apache.org/jira/browse/THRIFT-3191?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

James E. King, III updated THRIFT-3191:
---------------------------------------
    Description: 
While adding "make cross" test server support for some other refactoring I 
found that the generated code to handle testException exception responses does 
not work properly.  The test says that the code can die with the specified 
exceptions, but it can also die with Thrift::TException.

The generated code that fails in ThriftTest.pm:
{noformat}
sub process_testException {
    my ($self, $seqid, $input, $output) = @_;
    my $args = new ThriftTest::ThriftTest_testException_args();
    $args->read($input);
    $input->readMessageEnd();
    my $result = new ThriftTest::ThriftTest_testException_result();
    eval {
      $self->{handler}->testException($args->arg);
    }; if( UNIVERSAL::isa($@,'ThriftTest::Xception') ){ 
      $result->{err1} = $@;
    }
    $output->writeMessageBegin('testException', TMessageType::REPLY, $seqid);
    $result->write($output);
    $output->writeMessageEnd();
    $output->getTransport()->flush();
}
{noformat}

If the resulting implementation dies with a {{new Thrift::TException("foo")}}, 
the C++ client side gets a void back.
The result should be a TMessageType::EXCEPTION so that the client understands 
something went wrong at a more fundamental level than in the handler.

There are a number of other issues with "make cross", for example 
TApplicationException's constructor in perl doesn't pass the arguments to the 
TException::SUPER::new method.  All of these things need to be fixed in order 
for make cross to work.

  was:
While adding "make cross" test server support for some other refactoring I 
found that the generated code to handle testException exception responses does 
not work properly.  The test says that the code can die with the specified 
exceptions, but it can also die with Thrift::TException.

The generated code that fails in ThriftTest.pm:
{noformat}
sub process_testException {
    my ($self, $seqid, $input, $output) = @_;
    my $args = new ThriftTest::ThriftTest_testException_args();
    $args->read($input);
    $input->readMessageEnd();
    my $result = new ThriftTest::ThriftTest_testException_result();
    eval {
      $self->{handler}->testException($args->arg);
    }; if( UNIVERSAL::isa($@,'ThriftTest::Xception') ){ 
      $result->{err1} = $@;
    }
    $output->writeMessageBegin('testException', TMessageType::REPLY, $seqid);
    $result->write($output);
    $output->writeMessageEnd();
    $output->getTransport()->flush();
}
{noformat}

If the resulting implementation dies with a {{new Thrift::TException("foo")}}, 
the C++ client side gets a void back.

Code that allows the test to pass adds support for capturing TException:
{noformat}
    eval {
      $self->{handler}->testException($args->arg);
    }; if( UNIVERSAL::isa($@,'ThriftTest::Xception') ){ 
      $result->{err1} = $@;
    }; if( UNIVERSAL::isa($@,'Thrift::TException') ){ 
      $result->{err1} = $@;
    }
}
{noformat}

Adding this to the compiler output is reasonable, however what the generated 
code is doing when an exception of another type is thrown seems quite wrong.  
It simply ignores the exception and returns an empty reply.  I think it would 
make more sense here to catch all not-specifically generated exceptions and 
make a TException that describes what happened, but let the server continue 
processing.  This would be a programming error on the server handler 
implementation.  Given TException is a base class, we convert this into a 
TApplicationException with the code INTERNAL_ERROR.

We end up with:
{noformat}
    eval {
      $self->{handler}->testException($args->arg);
    }; if( UNIVERSAL::isa($@,'ThriftTest::Xception') ){ 
      $result->{err1} = $@;
    } elsif (defined $@ and not defined $result->{err1}) {
      $result->{err1} = new TApplicationException("Unexpected Exception: " . 
$@, TApplicationException::INTERNAL_ERROR);
      $@ = undef;
    }
{noformat}


> Perl compiler does not add support for unexpected exception handling
> --------------------------------------------------------------------
>
>                 Key: THRIFT-3191
>                 URL: https://issues.apache.org/jira/browse/THRIFT-3191
>             Project: Thrift
>          Issue Type: Bug
>          Components: Perl - Compiler
>    Affects Versions: 0.9.2
>            Reporter: James E. King, III
>            Assignee: James E. King, III
>            Priority: Critical
>
> While adding "make cross" test server support for some other refactoring I 
> found that the generated code to handle testException exception responses 
> does not work properly.  The test says that the code can die with the 
> specified exceptions, but it can also die with Thrift::TException.
> The generated code that fails in ThriftTest.pm:
> {noformat}
> sub process_testException {
>     my ($self, $seqid, $input, $output) = @_;
>     my $args = new ThriftTest::ThriftTest_testException_args();
>     $args->read($input);
>     $input->readMessageEnd();
>     my $result = new ThriftTest::ThriftTest_testException_result();
>     eval {
>       $self->{handler}->testException($args->arg);
>     }; if( UNIVERSAL::isa($@,'ThriftTest::Xception') ){ 
>       $result->{err1} = $@;
>     }
>     $output->writeMessageBegin('testException', TMessageType::REPLY, $seqid);
>     $result->write($output);
>     $output->writeMessageEnd();
>     $output->getTransport()->flush();
> }
> {noformat}
> If the resulting implementation dies with a {{new 
> Thrift::TException("foo")}}, the C++ client side gets a void back.
> The result should be a TMessageType::EXCEPTION so that the client understands 
> something went wrong at a more fundamental level than in the handler.
> There are a number of other issues with "make cross", for example 
> TApplicationException's constructor in perl doesn't pass the arguments to the 
> TException::SUPER::new method.  All of these things need to be fixed in order 
> for make cross to work.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to