Thrift
  1. Thrift
  2. THRIFT-1238

Thrift JS client cannot read map of structures

    Details

    • Patch Info:
      Patch Available

      Description

      Apparently we have a problem reading map of structures, only one item is been read and the other come as undefined in the result map on the client side.

      I was having this with some complex maps and notice that the test case 'testInsanity' was quite similar to my case but was actually been tested at all. Then checking its result I could see that's it actually a general problem.

      The server sends 2 elements:

      {
          "1":{
              "3":{
                  "userMap":{
                      "8":8,
                      "5":5
                  },
                  "xtructs":[{
                          "string_thing":"Goodbye4",
                          "byte_thing":4,
                          "i32_thing":4,
                          "i64_thing":4
                      },
                      {
                          "string_thing":"Hello2",
                          "byte_thing":2,
                          "i32_thing":2,
                          "i64_thing":2
                      }
                  ]
              },
              "2":{
                  "userMap":{
                      "8":8,
                      "5":5
                  },
                  "xtructs":[{
                          "string_thing":"Goodbye4",
                          "byte_thing":4,
                          "i32_thing":4,
                          "i64_thing":4
                      },
                      {
                          "string_thing":"Hello2",
                          "byte_thing":2,
                          "i32_thing":2,
                          "i64_thing":2
                      }
                  ]
              }
          },
          "2":{
              "6":{
                  "userMap":null,
                  "xtructs":null
              }
          }
      }
      

      but we can only read 1:

      {
          "1":{
              "2":{
                  "userMap":{
                      "8":8,
                      "5":5
                  },
                  "xtructs":[{
                          "string_thing":"Goodbye4",
                          "byte_thing":4,
                          "i32_thing":4,
                          "i64_thing":4
                      },
                      {
                          "string_thing":"Hello2",
                          "byte_thing":2,
                          "i32_thing":2,
                          "i64_thing":2
                      }
                  ]
              },
              "undefined":{
                  "userMap":null,
                  "xtructs":null
              }
          },
          "2":{
              "6":{
                  "userMap":null,
                  "xtructs":null
              }
          }
      }
      

      on my code it also happens with 3 or more structures, only the first one is read, the second is "undefined" and the rest is gone!

      ps.: I'm submitting the patch for the unit test. I had a quick look on the code but could find anything yet and won't have time in the next weeks...

      1. THRIFT-1238-qunit-insanity.patch
        3 kB
        Henrique Mendonça
      2. THRIFT-1238-fix-maps+qunit.patch
        4 kB
        Henrique Mendonça

        Issue Links

          Activity

          Hide
          Roger Meier added a comment -

          I committed the testcase, thanks Henrique!

          Show
          Roger Meier added a comment - I committed the testcase, thanks Henrique!
          Hide
          Hudson added a comment -

          Integrated in Thrift #193 (See https://builds.apache.org/job/Thrift/193/)
          THRIFT-1238 Thrift JS client cannot read map of structures(TestCase)
          Patch: Henrique Mendonca

          roger : http://svn.apache.org/viewvc/?view=rev&rev=1147301
          Files :

          • /thrift/trunk/lib/js/test/build.xml
          • /thrift/trunk/lib/js/test/test.html
          • /thrift/trunk/lib/java/test/org/apache/thrift/server/ServerTestBase.java
          Show
          Hudson added a comment - Integrated in Thrift #193 (See https://builds.apache.org/job/Thrift/193/ ) THRIFT-1238 Thrift JS client cannot read map of structures(TestCase) Patch: Henrique Mendonca roger : http://svn.apache.org/viewvc/?view=rev&rev=1147301 Files : /thrift/trunk/lib/js/test/build.xml /thrift/trunk/lib/js/test/test.html /thrift/trunk/lib/java/test/org/apache/thrift/server/ServerTestBase.java
          Hide
          Henrique Mendonça added a comment -

          finally: THRIFT-1238-fix-maps+qunit.patch
          Although I still see a weird behavior with lis<map<str, str>>, it should fix most of the problems with maps deserialization.

          Show
          Henrique Mendonça added a comment - finally: THRIFT-1238 -fix-maps+qunit.patch Although I still see a weird behavior with lis<map<str, str>>, it should fix most of the problems with maps deserialization.
          Hide
          Roger Meier added a comment -

          Thanks Henrique, committed!

          Show
          Roger Meier added a comment - Thanks Henrique, committed!
          Hide
          Hudson added a comment -

          Integrated in Thrift #256 (See https://builds.apache.org/job/Thrift/256/)
          THRIFT-1238 Thrift JS client cannot read map of structures
          Patch: Henrique Mendonca

          roger : http://svn.apache.org/viewvc/?view=rev&rev=1167032
          Files :

          • /thrift/trunk/compiler/cpp/src/generate/t_js_generator.cc
          • /thrift/trunk/lib/js/test/build.xml
          • /thrift/trunk/lib/js/test/test.js
          Show
          Hudson added a comment - Integrated in Thrift #256 (See https://builds.apache.org/job/Thrift/256/ ) THRIFT-1238 Thrift JS client cannot read map of structures Patch: Henrique Mendonca roger : http://svn.apache.org/viewvc/?view=rev&rev=1167032 Files : /thrift/trunk/compiler/cpp/src/generate/t_js_generator.cc /thrift/trunk/lib/js/test/build.xml /thrift/trunk/lib/js/test/test.js
          Hide
          Ryan Phillips added a comment -

          Hi!

          I'm testing thrift 0.8.0 with our code base and came across an issue using javascript and node-thrift.

          Our generated thrift code runs with this error: Uncaught TypeError: Cannot read property 'length' of undefined.

          The generated code includes:
          if (input.rstack.length > input.rpos[input.rpos.length -1] + 1)

          { input.rstack.pop(); }

          It appears this patch breaks binary protocol compatibility, but I want to run the issue by everyone, since I'm new with thrift.

          Thanks,
          Ryan

          Show
          Ryan Phillips added a comment - Hi! I'm testing thrift 0.8.0 with our code base and came across an issue using javascript and node-thrift. Our generated thrift code runs with this error: Uncaught TypeError: Cannot read property 'length' of undefined. The generated code includes: if (input.rstack.length > input.rpos [input.rpos.length -1] + 1) { input.rstack.pop(); } It appears this patch breaks binary protocol compatibility, but I want to run the issue by everyone, since I'm new with thrift. Thanks, Ryan
          Hide
          Roger Meier added a comment -

          Hi Ryan

          The JavaScript implementation does only support json protocol. The js test suite does cover most of the features.

          node js does not have a testsuite, see THRIFT-1134 and it does not use the same code base as we have within thrift.js
          The starting point for node.js bug fixes are the unit tests.

          A possible approach might be to introduce require.js on js side to share the same code basis for node and regular javascript.

          -roger

          Show
          Roger Meier added a comment - Hi Ryan The JavaScript implementation does only support json protocol. The js test suite does cover most of the features. node js does not have a testsuite, see THRIFT-1134 and it does not use the same code base as we have within thrift.js The starting point for node.js bug fixes are the unit tests. A possible approach might be to introduce require.js on js side to share the same code basis for node and regular javascript. -roger
          Hide
          Mars Hsu added a comment -

          I think this issus still not fixed. Here is a example

          enum AutoAttribute

          { POWER = 0, MANIPULATE = 1, APPEARANCE = 2, DURABILITY = 3, }

          struct UserAutoDTO

          { 1: required string autoId, 2: required string autoName, 3: required string color, 4: required map<AutoAttribute, double> currentAttribute, }

          the client is scala(java), and service side is nodejs.

          [2012-04-05 08:02:41.547] [INFO] console - input.rstack:
          [2012-04-05 08:02:41.548] [INFO] console - undefined
          [2012-04-05 08:02:41.549] [INFO] console - input.rpos:
          [2012-04-05 08:02:41.549] [INFO] console - undefined

          /web/node_modules/thrift/lib/thrift/server.js:50
          throw e;
          ^
          TypeError: Cannot read property 'length' of undefined
          at Object.read (/web/gen-nodejs/auto_types.js:1670:29)

          Show
          Mars Hsu added a comment - I think this issus still not fixed. Here is a example enum AutoAttribute { POWER = 0, MANIPULATE = 1, APPEARANCE = 2, DURABILITY = 3, } struct UserAutoDTO { 1: required string autoId, 2: required string autoName, 3: required string color, 4: required map<AutoAttribute, double> currentAttribute, } the client is scala(java), and service side is nodejs. [2012-04-05 08:02:41.547] [INFO] console - input.rstack: [2012-04-05 08:02:41.548] [INFO] console - undefined [2012-04-05 08:02:41.549] [INFO] console - input.rpos: [2012-04-05 08:02:41.549] [INFO] console - undefined /web/node_modules/thrift/lib/thrift/server.js:50 throw e; ^ TypeError: Cannot read property 'length' of undefined at Object.read (/web/gen-nodejs/auto_types.js:1670:29)
          Hide
          Henrique Mendonça added a comment -

          Hi Mars Hsu
          This ticket was created for the "browser" JavaScript thrift. Would you mind to create a new issue for this?
          Ryan could be right and this could actually be caused by my patch but I'm not very sure if it was working before either...
          I'm pretty busy at the moment but I'll be glad to test a patch if you have one

          Thanks,
          Henrique

          Show
          Henrique Mendonça added a comment - Hi Mars Hsu This ticket was created for the "browser" JavaScript thrift. Would you mind to create a new issue for this? Ryan could be right and this could actually be caused by my patch but I'm not very sure if it was working before either... I'm pretty busy at the moment but I'll be glad to test a patch if you have one Thanks, Henrique

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development