mikemintz/rethinkdb-websocket-server

Error: Uncaught SyntaxError: Unexpected token in JSON

coffenbacher opened this issue · 15 comments

We've been experiencing a weird bug off and on over the past few months, and lately it seems to be getting worse.

  • Symptom: a syntax error in JSON parsing on the client. It's like part of the JSON message gets dropped or mangled in transmission. For example:
Error: Uncaught SyntaxError: Unexpected token , in JSON at position 289587
  • Clients: Chrome on OS X and Windows. It seems to occur much more frequently on OS X, and possibly not at all on Windows (we haven't been able to repro on Windows)
  • Versions: rethinkdb-websocket-server v0.5.1, react-rethinkdb v0.5.1
  • It seems to occur much more frequently (possibly exclusively) when the returned object is big / complicated.
  • Sometimes it happens 50% of the time for big objects, sometimes 0%, sometimes 100% of the time
  • We're using wss://

I'm going to keep looking into this but I don't have many ideas about where to start. Does anything jump out?

Nothing jumps out immediately as a possible reason to me. When you say "sometimes 100% of the time", does that mean you have a situation that you can reproduce where it always happens?

Are you able to see what the query is and what the results are in which it happens? If possible, I'd also try tcpdump or wireshark to compare to see if there are any differences between 1) what the browser sees in the chrome dev console network tab, 2) what the browser actually receives, 3) what rethinkdb-websocket-server actually sends to browser, 4) what rethinkdb actually sends to rethinkdb-websocket-server. Then it should be clear at what point the JSON is becoming invalid.

Haha - I just realized "sometimes 100% of the time" is pretty ridiculous sounding. I meant that for periods of time (like 15 minutes to 1 hour) it will be happening every query. And for other periods of time it will happen periodically or not at all. I don't yet know how to solidly repro it so we're still experimenting. It doesn't help that I'm on Windows where it always works - I have to have my partner do it on his Mac.

Good idea - we were trying client-side wireshark but I'll try to correlate that with tcpdump outputs from the server as well. Thanks! I'll report back here if I find anything out, since this seems to be a rare but persistent issue...we've seen it on two projects now.

Same here.

JS Error: Uncaught SyntaxError: Unexpected token � in JSON at position 1179583

The json is invalid:

image

Looks like there should be a comma instead of � sign.

No idea how to fix this. This present on pretty big data only (>10 mb). However, you have ~10% chance to get the data correctly. Otherwise, you need to refresh the page to request it again.

Hey @jerrygreen, any luck fixing it? We've been stumped for like 6 months now... it's bizarre. My ~2015 Surface Pro 3 sees it like 100x less than my coworkers who have 2011 Macbooks, but even I've seen it a handful of times. There is 100% some relation to the client, I'm not sure whether it's Windows vs. OS X, Windows Chrome vs. OS X Chrome, slower CPU vs faster CPU, or what.

Also interesting info in #24

Hi all.

I've been experiencing this bug too, as described towards the end in #24.

I've done a little digging and have been able to locate the bug and fix it too :)

The bug is in the rethinkdb-websocket-client library. Deep down in the socket handling code a FileReader object is used to read binary blobs off the websocket. Thing is, the FileReader load-events are not guaranteed to be delivered sequentially, at least not in Chrome where I have been troubled. It seems smaller blobs load faster than larger blobs (see below).

So a websocket read batch that looks like this in the browser network console:

65424
65424
65424
65424
31354

would sometimes be delivered as follows by the Socket object:

65424
65424
65424
31354
65424

ouch! that breaks the JSON parsing for sure :)

I've made a patch for the Socket object defined in /dist/index.js (lineno 6000-ish ) to ensure sequential delivery, and verified that it sucessfully corrects out-of-order batches from the FileReader.

// Socket constructor variables
var count = 0;
var pending = {};
var stack = [];

// in ws.onmessage
if (typeof Blob !== 'undefined' && data instanceof Blob) {
   count += 1;
   var id = count;
   stack.push(id);
   (0, _blobToBuffer2['default'])(data, function (err, buffer) {
	if (err) {throw err;}
        pending[id] = buffer;
	// ensure events are emitted sequentially 
        var _id = stack[0];	  
        while (pending.hasOwnProperty(_id)) {
		  var buf = pending[_id];
		  emitter.emit('data', buf);
		  delete pending[_id];
		  stack.shift();
		  if (stack.length === 0) {
		    	// no more elements
		    	break;
		  } else {
		      _id = stack[0];
		  }
        } 
    });
}

Cheers, Ingar

Amazing news (if true)!!! We will adopt the patch to see if it helps. Sounds very promising! Thank you for the investigation & patch, this has plagued us for a long time

@coffenbacher I don't see any fixes, what is it about?

Also, I have no fixes from myself. One solution in mind is to request smaller packs of data. Not megabytes. But generally yea... Better to fix the library. (Of course, I won't even try to do it.)

@jerrygreen did you see the post from @ingararntzen ? I haven't tested it yet but it certainly looks like a fix, it's certainly consistent with the behavior my team has been seeing

@ingararntzen oh my gosh! This really helps. Awesome!

@coffenbacher thank you too. Didn't notice this comment. The fix is not a lie.

UPD. actually, it is still possible to get the same error. However, looks like this appear less frequently

It does help a lot, although I very occasionally manage to get the error as well. Hopefully we can get this merged into the npm version, as it's much improved.

I'm happy to merge in this fix (or accept any PR), but I'm concerned that the error is still happening even after this fix. @coffenbacher or @jerrygreen, have you tried running with wsProtocols: 'base64'? That should completely bypass the asynchronous blobToBuffer code. And if the error is still occurring, then it will help narrow down where to look.

Yeah it's not a good long term solution (other than for clients like react-native that don't support binary), but it'll help pin down this bug.

Performance wise, it should use 30% more bandwidth. It'll take additional time to encode and decode, but I think it probably pales in comparison to the JSON encoding/decoding that has to happen anyway.

warmcat/libwebsockets#593
may be able to help you