Double JSON encoding with sockjs #79

Open
brycekahle opened this Issue Nov 8, 2013 · 7 comments

Projects

None yet

4 participants

@brycekahle

We are using primus with sockjs. It appears that our objects are getting encoded by both Primus and SockJS, so that a message on the websocket looks like this ["{\"action\":\"match\",\"id\":732427}"] instead of ["{"action":"match","id":732427}"].

I tried to implement a pass-through parser with this code:

parser: {
      encoder: function (data, fn) {
        fn(null, data);
      }
    , decoder: function (data, fn) {
        fn(null, data);
      }
    }

But I end up receiving the following message instead ["[object Object]"].

Any ideas on how I can avoid the inefficiency of calling JSON.stringify twice and the extra data?

@3rd-Eden
Primus member

Thanks for reporting the issue.

I think that SockJS is JSON encoding everything by default, even if it's a string. I'll see if we could maybe override something to make this logic a bit smarter. I'm gonna look more closer at it after Node Knockout.

@dignifiedquire

@3rd-Eden any updates on this? Just ran into this issues and it stops me from continuing my work. I can try fixing it but if you have any pointers/suggestions that'd be great.

@3rd-Eden
Primus member

@Dignifiedquire I know that @brycekahle got around it by using a custom fork of SockJS and a patch to the sockjs client that is bundled in Primus. This was the only way to go around it at the moment.

The biggest problem here is that SockJS does a forced toString in the client when you pass it a plain object: brycekahle@ddca639 so we cannot leverage the default JSON encoding of SockJS. So if you where to use SockJS normally, you would also need to double JSON encode your data.

@dignifiedquire

@3rd-Eden thanks, and onto using more forked versions of all the libraries :/

@brycekahle

@Dignifiedquire It is worth noting that in addition to the 2 patches I had to make, I also had to specify a passthrough parser in Primus. Like so:

 var primus = new Primus(server, {
      transformer: 'sockjs'
    , parser: {
        encoder: function (data, fn) {
          fn(null, data);
        }
      , decoder: function (data, fn) {
          fn(null, data);
        }
      }
    });
@brycekahle brycekahle referenced this issue in sockjs/sockjs-client Oct 28, 2014
Open

SockJS framing #205

@lpinca lpinca added the sockjs label Mar 5, 2015
@3rd-Eden
Primus member

Is this still an issue now that we're using the beta version of SockJS?

@brycekahle

Unfortunately yes. I couldn't find a clean way of fixing this without changing both server and client. I really would like to change the over-the-wire framing that SockJS uses to handle this and some other corner cases better, but that is a much larger change. See sockjs/sockjs-client#205

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment