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-read-undefined-exceptions2.patch
        2 kB
        Henrique Mendonça
      3. THRIFT-1065-throw-appexception.patch
        0.9 kB
        Henrique Mendonça

        Issue Links

          Activity

          Hide
          T Jake Luciani added a comment -

          Can you provide an example?

          Show
          T Jake Luciani added a comment - Can you provide an example?
          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
          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
          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
          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
          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

            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