Thrift
  1. Thrift
  2. THRIFT-1065

Unexpected exceptions not proper handled on JS

    Details

    • Patch Info:
      Patch Available

      Description

      Following THRIFT-550 and THRIFT-807:
      We still get JS errors when any undeclared exception is thrown (if not defined on the service IDL)

      1. THRIFT-1065-qunit-clean-up.patch
        2 kB
        Henrique Mendonça
      2. THRIFT-1065-throw-appexception.patch
        0.9 kB
        Henrique Mendonça
      3. THRIFT-1065-read-undefined-exceptions2.patch
        2 kB
        Henrique Mendonça

        Issue Links

          Activity

          This list of reviews may be incomplete, as errors occurred retrieving data from the following repositories:

          • Request to https://fisheye6.atlassian.com/ failed: Error in remote call to 'FishEye 0 (https://fisheye6.atlassian.com/)' (https://fisheye6.atlassian.com) [AbstractRestCommand{path='rest-service/search-v1/reviews', params={maxReturn=50, term=THRIFT-1065}, methodType=GET}] : Received status code 503 (Service Temporarily Unavailable)
          • Request to https://fisheye6.atlassian.com/ failed: Error in remote call to 'FishEye 0 (https://fisheye6.atlassian.com/)' (https://fisheye6.atlassian.com) [AbstractRestCommand{path='/rest-service-fe/search-v1/crossRepositoryQuery', params={expand=changesets[-100:-1], query=THRIFT-1065}, methodType=GET}] : Received status code 503 (Service Temporarily Unavailable)

          This list may be incomplete, as errors occurred whilst retrieving source from linked applications:

          • Request to https://fisheye6.atlassian.com/ failed: Error in remote call to 'FishEye 0 (https://fisheye6.atlassian.com/)' (https://fisheye6.atlassian.com) [AbstractRestCommand{path='/rest-service-fe/search-v1/crossRepositoryQuery', params={expand=changesets[0:20].revisions[0:29],reviews, query=THRIFT-1065}, methodType=GET}] : Received status code 503 (Service Temporarily Unavailable)
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Resolved Resolved
          5d 2h 33m 1 Roger Meier 22/Feb/11 21:04
          Resolved Resolved Closed Closed
          14d 21h 13m 1 Bryan Duxbury 09/Mar/11 18:17
          Bryan Duxbury made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Roger Meier made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Assignee Henrique Mendonca [ henrique ]
          Resolution Fixed [ 1 ]
          Hide
          Roger Meier added a comment -

          Yes Henrique, that's the way!

          I Just committed the following stuff:

          • qUnit test, modified a bit
          • JavaServer Handler, modified to the same behavior as C++ (see THRIFT-847)
          • original JavaScript patch

          Thank you for making Thrift better!

          Roger

          Show
          Roger Meier added a comment - Yes Henrique, that's the way! I Just committed the following stuff: qUnit test, modified a bit JavaServer Handler, modified to the same behavior as C++ (see THRIFT-847 ) original JavaScript patch Thank you for making Thrift better! Roger
          Hide
          Roger Meier added a comment -

          behavior of Java and CPP Testframework should be the same

          Show
          Roger Meier added a comment - behavior of Java and CPP Testframework should be the same
          Roger Meier made changes -
          Link This issue is part of THRIFT-847 [ THRIFT-847 ]
          Henrique Mendonça made changes -
          Attachment THRIFT-1065-throw-appexception.patch [ 12471652 ]
          Attachment THRIFT-1065-qunit-clean-up.patch [ 12471653 ]
          Hide
          Henrique Mendonça added a comment -

          apparently the java server is not encoding undeclared exceptions "properly" (it could be my impl., please see patch), but it works fine on C++

          Incoming content: [1,"testException",1,0,{"1":{"str":"TApplicationException"}}]
          [cpp] Outgoing content: [1,"testException",3,0,{"1":

          {"str":"This is a TApplicationException"}

          ,"2":{"i32":0}}]
          [java] Outgoing content: Error:This is a TApplicationException

          Show
          Henrique Mendonça added a comment - apparently the java server is not encoding undeclared exceptions "properly" (it could be my impl., please see patch), but it works fine on C++ Incoming content: [1,"testException",1,0,{"1":{"str":"TApplicationException"}}] [cpp] Outgoing content: [1,"testException",3,0,{"1": {"str":"This is a TApplicationException"} ,"2":{"i32":0}}] [java] Outgoing content: Error:This is a TApplicationException
          Hide
          Henrique Mendonça added a comment - - edited

          e.g. if you remove any throws declaration from ThriftTest.thrift:
          void testException(1: string arg) throws(1: Xception err1),
          void testException(1: string arg),

          and call the service from the JS client side. The server throws then the "unexpected" exception and I was getting something like:

          fname not defined
          var ret = input.readStructBegin(fname)

          Now, with this patch, it should still throw the exception (TApplicationException) on the JS client site

          Show
          Henrique Mendonça added a comment - - edited e.g. if you remove any throws declaration from ThriftTest.thrift: void testException(1: string arg) throws(1: Xception err1), void testException(1: string arg), and call the service from the JS client side. The server throws then the "unexpected" exception and I was getting something like: fname not defined var ret = input.readStructBegin(fname) Now, with this patch, it should still throw the exception (TApplicationException) on the JS client site
          Henrique Mendonça made changes -
          Field Original Value New Value
          Attachment THRIFT-1065-read-undefined-exceptions2.patch [ 12471304 ]
          Hide
          Henrique Mendonça added a comment -

          remove some misplaced code (probably copied from another implementation?)

          Show
          Henrique Mendonça added a comment - remove some misplaced code (probably copied from another implementation?)
          Hide
          T Jake Luciani added a comment -

          Can you provide an example?

          Show
          T Jake Luciani added a comment - Can you provide an example?
          Henrique Mendonça created issue -

            People

            • Assignee:
              Henrique Mendonça
              Reporter:
              Henrique Mendonça
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development