Thrift
  1. Thrift
  2. THRIFT-1297

Compiled javascript code does not write falsy values

    Details

      Description

      I compiled cassandra.thrift using "thrift --gen js:node cassandra.thrift".

      When I use this code this results in the following error from cassandra / thrift:

      { name: 'TApplicationException',  type: 7,
        message: 'Required field \'reversed\' was not found in serialized data! Struct : SliceRange(start:null, finish:null, reversed:false, count:100)' }
      

      If I look in the generated code, I find

      SliceRange.prototype.write = function(output) {
        output.writeStructBegin('SliceRange');
        if (this.start) {
          output.writeFieldBegin('start', Thrift.Type.STRING, 1);
          output.writeString(this.start);
          output.writeFieldEnd();
        }
        if (this.finish) {
          output.writeFieldBegin('finish', Thrift.Type.STRING, 2);
          output.writeString(this.finish);
          output.writeFieldEnd();
        }
        if (this.reversed) {
          output.writeFieldBegin('reversed', Thrift.Type.BOOL, 3);
          output.writeBool(this.reversed);
          output.writeFieldEnd();
        }
        if (this.count) {
          output.writeFieldBegin('count', Thrift.Type.I32, 4);
          output.writeI32(this.count);
          output.writeFieldEnd();
        }
        output.writeFieldStop();
        output.writeStructEnd();
        return;
      };
      

      this.reversed is false and subsequently does not get sent
      If I look at the compiled version that was generated by 0.6.1 I see:

      SliceRange.prototype.write = function(output){ 
        output.writeStructBegin('SliceRange')
        if (null != this.start) {
          output.writeFieldBegin('start', Thrift.Type.STRING, 1)
          output.writeString(this.start)
          output.writeFieldEnd()
        }
        if (null != this.finish) {
          output.writeFieldBegin('finish', Thrift.Type.STRING, 2)
          output.writeString(this.finish)
          output.writeFieldEnd()
        }
        if (null != this.reversed) {
          output.writeFieldBegin('reversed', Thrift.Type.BOOL, 3)
          output.writeBool(this.reversed)
          output.writeFieldEnd()
        }
        if (null != this.count) {
          output.writeFieldBegin('count', Thrift.Type.I32, 4)
          output.writeI32(this.count)
          output.writeFieldEnd()
        }
        output.writeFieldStop()
        output.writeStructEnd()
        return
      }
      

      null is also not be completely correct, the solution would be:

      ...
      if (undefined !== this.reversed)
      ...
      

      Unless this change was intentional and breaks things because cassandra is perhaps using an old thrift version?

        Issue Links

          Activity

          Hide
          Jake Farrell added a comment -

          Is this the same issue as THRIFT-1277 ?

          Show
          Jake Farrell added a comment - Is this the same issue as THRIFT-1277 ?
          Hide
          Joris van der Wel added a comment -

          I am not sure what is considered null in thrift.

          My issue is that .write is not called at all when the expression is falsy (null, false, undefined, 0, '', NaN). I think the value is not sent over the network at all. If that is considered as null by the receiving end, it could be the same issue.

          If null and a non existing value are not considered the same, it is a different issue and strict equality would be needed (!== ===)

          Show
          Joris van der Wel added a comment - I am not sure what is considered null in thrift. My issue is that .write is not called at all when the expression is falsy (null, false, undefined, 0, '', NaN). I think the value is not sent over the network at all. If that is considered as null by the receiving end, it could be the same issue. If null and a non existing value are not considered the same, it is a different issue and strict equality would be needed (!== ===)
          Hide
          Joris van der Wel added a comment -

          I just ran into this again and spent a day tracking down what was going on.
          I noticed some more things:

          Setting a value to falsy, results in the value not being sent over the network. If this value is required, it results in an exception on the server (c++ non blocking) which closes the socket.

          It also happens for function parameters. On my c++ server, thrift ended up giving me an uninitialized value, so the effect was seemingly random.

          I currently work around it by using new Number(0), new String(''), etc (which are not falsy).
          But this bug pretty much breaks thrifts node.js support

          Show
          Joris van der Wel added a comment - I just ran into this again and spent a day tracking down what was going on. I noticed some more things: Setting a value to falsy, results in the value not being sent over the network. If this value is required, it results in an exception on the server (c++ non blocking) which closes the socket. It also happens for function parameters. On my c++ server, thrift ended up giving me an uninitialized value, so the effect was seemingly random. I currently work around it by using new Number(0), new String(''), etc (which are not falsy). But this bug pretty much breaks thrifts node.js support
          Hide
          Kenny added a comment -

          Thanks for the workaround Joris. This is still broken. I finally ran WireShark captures to see why my i32s were ending up 'undefined' in Erlang when passed as 0.

          Called getEvents(___, 0):
          [1,"getEvents",1,0,{"1":{"i64":7426226942022814}}]

          Called getEvents(___, 1):
          [1,"getEvents",1,0,{"1":

          {"i64":7426226942022814}

          ,"2":{"i32":1}}]

          Called getEvents(___, new Number(0):
          [1,"getEvents",1,0,{"1":

          {"i64":7426226942022814}

          ,"2":{"i32":0}}]

          Show
          Kenny added a comment - Thanks for the workaround Joris. This is still broken. I finally ran WireShark captures to see why my i32s were ending up 'undefined' in Erlang when passed as 0. Called getEvents(___, 0): [1,"getEvents",1,0,{"1":{"i64":7426226942022814}}] Called getEvents(___, 1): [1,"getEvents",1,0,{"1": {"i64":7426226942022814} ,"2":{"i32":1}}] Called getEvents(___, new Number(0): [1,"getEvents",1,0,{"1": {"i64":7426226942022814} ,"2":{"i32":0}}]
          Hide
          Larregoity added a comment -

          Thanks for the work around.
          Unfortunately, for booleans, wrapping using new Boolean(bool) this is not enough:

          var bool = false;
          new Boolean(bool) is not falsy, so it will be written.

          But as in lib.js, the writeBool method is:
          writeBool: function(value)

          { this.tstack.push(value ? 1 : 0); }

          ,
          The value pushed on the wire will always be 1.

          I work around this issue using:

          var oldWriteBool = Thrift.Protocol.prototype.writeBool;
          var newWriteBool = function (value) {
          var fixedValue = value;
          if (value instanceof Boolean)

          { fixedValue = value.valueOf(); }

          oldWriteBool.call(this, fixedValue);
          }
          Thrift.Protocol.prototype.writeBool = newWriteBool;

          and then I can pass false booleans by wrapping them into new Boolean.

          Show
          Larregoity added a comment - Thanks for the work around. Unfortunately, for booleans, wrapping using new Boolean(bool) this is not enough: var bool = false; new Boolean(bool) is not falsy, so it will be written. But as in lib.js, the writeBool method is: writeBool: function(value) { this.tstack.push(value ? 1 : 0); } , The value pushed on the wire will always be 1. I work around this issue using: var oldWriteBool = Thrift.Protocol.prototype.writeBool; var newWriteBool = function (value) { var fixedValue = value; if (value instanceof Boolean) { fixedValue = value.valueOf(); } oldWriteBool.call(this, fixedValue); } Thrift.Protocol.prototype.writeBool = newWriteBool; and then I can pass false booleans by wrapping them into new Boolean.
          Hide
          Christoph Tavan added a comment -

          After some research I think this one is also a duplicate of THRIFT-1277.

          Show
          Christoph Tavan added a comment - After some research I think this one is also a duplicate of THRIFT-1277 .

            People

            • Assignee:
              Henrique Mendonça
              Reporter:
              Joris van der Wel
            • Votes:
              3 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development