megastef / node-crate

Node.js base DB-Driver for CRATE (www.crate.io)
52 stars 18 forks source link

join without any char #8

Closed michabbb closed 9 years ago

michabbb commented 9 years ago

join with a dot produces extra comma and destroys valid json

megastef commented 9 years ago

Hi, which test fails? Which node.js version and Crate version do you use?

michabbb commented 9 years ago

test ? real life, i was debugging hours until i got to this section where this happens:

result = JSON.parse(data.join("."));

i did a query on crate and was very surprised that randomly an error occurs like:

SyntaxError: Unexpected token .
SyntaxError: Unexpected token ,
SyntaxError: Unexpected token ]
SyntaxError: Unexpected token [

i was debugging the raw result of the crate SQL endpoint: all fine so the result of crate is not the problem - thank´s god.

so i replaced

data.push(chunk);

with

data += chunk;

and made data to a string and noticed, that there is no error anymore. the join gives me an extra comma - so the result json is broken, because at a random (don´t know why random, i guess the chunk is not always the same size) position i have a comma that does not belong here.

this whole thing is 100% reproducable for me.

here are my specs:

martinheidegger commented 9 years ago

First off: Thank you for investing your time into it. It super annoying to run into this sort of problem.

There is a problem with this PR however: This PR should contain a unit test that fails that other people can reproduce as well in order for it not to reappear at a later point in time.

michabbb commented 9 years ago

np! sorry, i am not that familar with node and have never written a unit test for it, so i cannot provide this. but yes, this kind of problem is a big piec of sh*\ because you will never think of something like this ;-) thanks for merging so fast, so i can go in my work ;)

megastef commented 9 years ago

The error sounds logic, but I have never seen the problem. I guess it appears only for large result sets, if one puts high 'limit' in queries and data arrives then really in multiple chunks.

Thanks for reporting it.