[
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.
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}
In addition to this, TException needs to be able to write itself out because it
is not generated, but it doesn't define a write method.
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.
We end up with:
{noformat}
eval {
$self->{handler}->testException($args->arg);
}; if( UNIVERSAL::isa($@,'ThriftTest::Xception') ){
$result->{err1} = $@;
} elsif (defined $@) {
$result->{err1} = new Thrift::TException("Unexpected exception: ".$@);
}
{noformat}
In addition to this, TException needs to be able to write itself out because it
is not generated, but it doesn't define a write method.
> 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.
> 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}
> In addition to this, TException needs to be able to write itself out because
> it is not generated, but it doesn't define a write method.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
