Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-3191

Perl compiler does not add support for unexpected exception handling

VotersWatch issueWatchersLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Bug
    • Status: Closed
    • Critical
    • Resolution: Fixed
    • 0.9.2
    • 0.9.3
    • Perl - Compiler
    • None

    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:

      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();
      }
      

      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.

      Attachments

        Issue Links

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            jking3 James E. King III
            jking3 James E. King III
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment