From c21b886a17f76b901430a9522b7cd6682f4849bf Mon Sep 17 00:00:00 2001 From: Carsten Mjartan Date: Thu, 22 Dec 2016 10:06:40 +0100 Subject: [PATCH 1/9] Fix for #112 --- index.js | 4 ++++ test/issues.js | 22 ++++++++++++++++++++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index c164100..7448b4a 100755 --- a/index.js +++ b/index.js @@ -23,6 +23,9 @@ exports.parse = function (path, map) { parser.write(chunk) }, function (data) { + if (parser.tState != Parser.C.START) { + stream.emit('error', new Error('Incomplete JSON')); + } if(data) stream.write(data) if (header) @@ -30,6 +33,7 @@ exports.parse = function (path, map) { if (footer) stream.emit('footer', footer) stream.queue(null) + console.log(`State: ${parser.tState}`); }) if('string' === typeof path) diff --git a/test/issues.js b/test/issues.js index ea4c743..c6d2ae9 100644 --- a/test/issues.js +++ b/test/issues.js @@ -6,11 +6,11 @@ test('#66', function (t) { var stream = JSONStream .parse() .on('error', function (err) { - t.ok(err); + t.ok(err, "error emitted" + err.message); error++; }) .on('end', function () { - t.ok(error === 1); + t.ok(error === 2, "error called 2 times before end"); t.end(); }); @@ -19,6 +19,24 @@ test('#66', function (t) { }); +test('#112', function (t) { + var error = 0; + var stream = JSONStream + .parse() + .on('error', function (err) { + t.ok(err, "error emitted: " + err.message); + error++; + }) + .on('end', function () { + t.ok(error === 1, "error called before end"); + t.end(); + }); + + stream.write('{"rows":[{"id":"id-1","name":"Name A"},{"id":"id-2","name":"'); + stream.end(); + +}); + test('#81 - failure to parse nested objects', function (t) { var stream = JSONStream .parse('.bar.foo') From 3b9fa94dd370162cf2e4422f0258e9f5d221bc8a Mon Sep 17 00:00:00 2001 From: Carsten Mjartan Date: Thu, 22 Dec 2016 10:07:36 +0100 Subject: [PATCH 2/9] Remove console info --- index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/index.js b/index.js index 7448b4a..a6aa9c8 100755 --- a/index.js +++ b/index.js @@ -33,7 +33,6 @@ exports.parse = function (path, map) { if (footer) stream.emit('footer', footer) stream.queue(null) - console.log(`State: ${parser.tState}`); }) if('string' === typeof path) From 7fed6856ff146cbb7125187983967aa7a0702ad7 Mon Sep 17 00:00:00 2001 From: Carsten Mjartan Date: Thu, 22 Dec 2016 15:04:04 +0100 Subject: [PATCH 3/9] Render header/footer first if they exist, then fail before end --- index.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index a6aa9c8..ef9e7ee 100755 --- a/index.js +++ b/index.js @@ -23,15 +23,16 @@ exports.parse = function (path, map) { parser.write(chunk) }, function (data) { - if (parser.tState != Parser.C.START) { - stream.emit('error', new Error('Incomplete JSON')); - } if(data) stream.write(data) if (header) stream.emit('header', header) if (footer) stream.emit('footer', footer) + + if (parser.tState != Parser.C.START) { + stream.emit('error', new Error('Incomplete JSON')); + } stream.queue(null) }) From 7559580527c97fe58728b07a2526a26a267481cc Mon Sep 17 00:00:00 2001 From: Carsten Mjartan Date: Thu, 20 Apr 2017 09:47:17 +0200 Subject: [PATCH 4/9] Fix end-after-error and duplicate-error behaviour --- index.js | 12 +++++-- test/issues.js | 92 ++++++++++++++++++++++++++++++++------------------ 2 files changed, 69 insertions(+), 35 deletions(-) diff --git a/index.js b/index.js index b63daff..8ed189e 100755 --- a/index.js +++ b/index.js @@ -18,22 +18,28 @@ exports.parse = function (path, map) { var header, footer var parser = new Parser() var stream = through(function (chunk) { + console.log("Called write"); if('string' === typeof chunk) chunk = new Buffer(chunk) parser.write(chunk) }, function (data) { + console.log("Called end"); if(data) stream.write(data) if (header) - stream.emit('header', header) + stream.emit('header', header) if (footer) stream.emit('footer', footer) if (parser.tState != Parser.C.START) { - stream.emit('error', new Error('Incomplete JSON')); + stream.emit('error', new Error('Incomplete JSON')) + if(!stream.writable && stream.autoDestroy) { + stream.destroy() + } + } else { + stream.queue(null) } - stream.queue(null) }) if('string' === typeof path) diff --git a/test/issues.js b/test/issues.js index c6d2ae9..9c9b203 100644 --- a/test/issues.js +++ b/test/issues.js @@ -1,52 +1,80 @@ -var JSONStream = require('../'); +var JSONStream = require('../') var test = require('tape') test('#66', function (t) { - var error = 0; + var error = 0 var stream = JSONStream - .parse() - .on('error', function (err) { - t.ok(err, "error emitted" + err.message); - error++; - }) - .on('end', function () { - t.ok(error === 2, "error called 2 times before end"); - t.end(); - }); - - stream.write('["foo":bar['); - stream.end(); + .parse() + .on('error', function (err) { + t.ok(err, "error emitted: " + err.message + "\n" + err.stack) + error++ + }) + .on('close', function() { + t.ok(error === 2, "expect error to be called twice ('Invalid JSON', 'Incomplete JSON'): " + error) + t.end() + }) + stream.write('["foo":bar[') + stream.end() +}) -}); +test('#112 should allow aborting after first error', function (t) { + var error = 0 + var stream = JSONStream + .parse() + .on('error', function (err) { + t.ok(err, "error emitted: " + err.message + "\n" + err.stack) + error++ + stream.destroy() + }) + .on('close', function() { + t.ok(error === 1, "expect error to be called once: " + error) + t.end() + }) + stream.write('["foo":bar[') + stream.end() +}) -test('#112', function (t) { - var error = 0; +test('#112 "Incomplete JSON" error is emitted', function (t) { var stream = JSONStream .parse() .on('error', function (err) { - t.ok(err, "error emitted: " + err.message); - error++; + t.ok("error emitted: " + err.message) + t.end() }) - .on('end', function () { - t.ok(error === 1, "error called before end"); - t.end(); - }); - stream.write('{"rows":[{"id":"id-1","name":"Name A"},{"id":"id-2","name":"'); - stream.end(); + stream.write('{"rows":[{"id":"id-1","name":"Name A"},{"id":"id-2","name":"') + stream.end() +}) -}); +test('#112 end is not emmitted after error', function (t) { + var ended = 0 + var stream = JSONStream + .parse() + .on('error', function (err) { + t.ok(err, "error emitted: " + err.message) + }) + .on('end', function () { + ended = 1 + }) + .on('close', function() { + t.ok(ended === 0 , "`end` emitted despite error") + t.end() + }) + + stream.write('{"rows":[{"id":"id-1","name":"Name A"},{"id":"id-2","name":"') + stream.end() +}) test('#81 - failure to parse nested objects', function (t) { var stream = JSONStream .parse('.bar.foo') .on('error', function (err) { - t.error(err); + t.error(err) }) .on('end', function () { - t.end(); - }); + t.end() + }) - stream.write('{"bar":{"foo":"baz"}}'); - stream.end(); -}); + stream.write('{"bar":{"foo":"baz"}}') + stream.end() +}) From c644ad58d6fe1bb609520bc0ddc0cb702df47618 Mon Sep 17 00:00:00 2001 From: Carsten Mjartan Date: Thu, 20 Apr 2017 10:41:42 +0200 Subject: [PATCH 5/9] Removed log messages --- index.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/index.js b/index.js index 8ed189e..6dab981 100755 --- a/index.js +++ b/index.js @@ -18,13 +18,11 @@ exports.parse = function (path, map) { var header, footer var parser = new Parser() var stream = through(function (chunk) { - console.log("Called write"); if('string' === typeof chunk) chunk = new Buffer(chunk) parser.write(chunk) }, function (data) { - console.log("Called end"); if(data) stream.write(data) if (header) From 40aeed2abad2c01f895e86e3a498a8df50b8fae4 Mon Sep 17 00:00:00 2001 From: Carsten Mjartan Date: Thu, 20 Apr 2017 11:01:52 +0200 Subject: [PATCH 6/9] Add nextTick for destroy(), improve readability --- index.js | 4 +++- test/destroy_missing.js | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/index.js b/index.js index 6dab981..05a3bb9 100755 --- a/index.js +++ b/index.js @@ -33,7 +33,9 @@ exports.parse = function (path, map) { if (parser.tState != Parser.C.START) { stream.emit('error', new Error('Incomplete JSON')) if(!stream.writable && stream.autoDestroy) { - stream.destroy() + process.nextTick(function () { + stream.destroy() + }); } } else { stream.queue(null) diff --git a/test/destroy_missing.js b/test/destroy_missing.js index 315fdc8..dbf1691 100644 --- a/test/destroy_missing.js +++ b/test/destroy_missing.js @@ -5,18 +5,18 @@ var file = join(__dirname, 'fixtures','all_npm.json'); var JSONStream = require('../'); -var server = net.createServer(function(client) { +var server = net.createServer(function(connection) { var parser = JSONStream.parse([]); parser.on('end', function() { console.log('close') console.error('PASSED'); server.close(); }); - client.pipe(parser); + connection.pipe(parser); var n = 4 - client.on('data', function () { + connection.on('data', function () { if(--n) return - client.end(); + connection.end(); }) }); server.listen(9999); From e3a0fee779cbebd83ce6adaf601235caf78312c4 Mon Sep 17 00:00:00 2001 From: Carsten Mjartan Date: Thu, 20 Apr 2017 11:19:05 +0200 Subject: [PATCH 7/9] Use random ports for tests --- test/destroy_missing.js | 10 +++++++--- test/multiple_objects.js | 12 ++++++++---- test/multiple_objects_error.js | 12 ++++++++---- 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/test/destroy_missing.js b/test/destroy_missing.js index dbf1691..f5c2c21 100644 --- a/test/destroy_missing.js +++ b/test/destroy_missing.js @@ -19,9 +19,13 @@ var server = net.createServer(function(connection) { connection.end(); }) }); -server.listen(9999); +server.listen(() => { + var port = server.address().port -var client = net.connect({ port : 9999 }, function() { - fs.createReadStream(file).pipe(client).on('data', console.log) //.resume(); + console.log('Listening on port ' + port) + var client = net.connect({ port : port }, function() { + fs.createReadStream(file).pipe(client).on('data', console.log) //.resume(); + }); }); + diff --git a/test/multiple_objects.js b/test/multiple_objects.js index 22f6324..e029b4c 100644 --- a/test/multiple_objects.js +++ b/test/multiple_objects.js @@ -28,9 +28,13 @@ var server = net.createServer(function(client) { }); client.pipe(parser); }); -server.listen(9999); -var client = net.connect({ port : 9999 }, function() { - var msgs = str + ' ' + str + '\n\n' + str - client.end(msgs); +server.listen(() => { + var port = server.address().port + + console.log('Listening on port ' + port) + var client = net.connect({ port : port }, function() { + var msgs = str + ' ' + str + '\n\n' + str + client.end(msgs); + }); }); diff --git a/test/multiple_objects_error.js b/test/multiple_objects_error.js index 83d113b..54b320e 100644 --- a/test/multiple_objects_error.js +++ b/test/multiple_objects_error.js @@ -21,9 +21,13 @@ var server = net.createServer(function(client) { }); client.pipe(parser); }); -server.listen(9999); -var client = net.connect({ port : 9999 }, function() { - var msgs = str + '}'; - client.end(msgs); +server.listen(() => { + var port = server.address().port + + console.log('Listening on port ' + port) + var client = net.connect({ port : port }, function() { + var msgs = str + '}'; + client.end(msgs); + }); }); From 4ede88fc4da8e94993a0d982b54f153eb3a8d3c7 Mon Sep 17 00:00:00 2001 From: Carsten Mjartan Date: Thu, 20 Apr 2017 12:37:09 +0200 Subject: [PATCH 8/9] Check with close event, as `end` is not emitted any more after error --- test/destroy_missing.js | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/test/destroy_missing.js b/test/destroy_missing.js index f5c2c21..0740a4a 100644 --- a/test/destroy_missing.js +++ b/test/destroy_missing.js @@ -4,19 +4,19 @@ var join = require('path').join; var file = join(__dirname, 'fixtures','all_npm.json'); var JSONStream = require('../'); - var server = net.createServer(function(connection) { var parser = JSONStream.parse([]); - parser.on('end', function() { + parser.on('close', function() { console.log('close') console.error('PASSED'); server.close(); }); - connection.pipe(parser); - var n = 4 + parser.on('error', function (err) { + console.log('Parser error as expected: ' + err.message) + }) + connection.pipe(parser) connection.on('data', function () { - if(--n) return - connection.end(); + connection.end(); }) }); @@ -27,5 +27,4 @@ server.listen(() => { var client = net.connect({ port : port }, function() { fs.createReadStream(file).pipe(client).on('data', console.log) //.resume(); }); -}); - +}); \ No newline at end of file From f420f157612908bf4b83813e4689e86c45eb8987 Mon Sep 17 00:00:00 2001 From: Carsten Mjartan Date: Fri, 14 Jul 2017 07:30:37 +0200 Subject: [PATCH 9/9] Also check completeness via stack size --- index.js | 2 +- test/issues.js | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index 05a3bb9..2b8275f 100755 --- a/index.js +++ b/index.js @@ -30,7 +30,7 @@ exports.parse = function (path, map) { if (footer) stream.emit('footer', footer) - if (parser.tState != Parser.C.START) { + if (parser.tState != Parser.C.START || parser.stack.length > 0) { stream.emit('error', new Error('Incomplete JSON')) if(!stream.writable && stream.autoDestroy) { process.nextTick(function () { diff --git a/test/issues.js b/test/issues.js index 9c9b203..3823ffc 100644 --- a/test/issues.js +++ b/test/issues.js @@ -46,6 +46,18 @@ test('#112 "Incomplete JSON" error is emitted', function (t) { stream.end() }) +test('#112 "Incomplete JSON" error is emitted with different JSON', function (t) { + var stream = JSONStream + .parse() + .on('error', function (err) { + t.ok("error emitted: " + err.message) + t.end() + }) + + stream.write('{"rows":[{"id":"id-1","name":"Name A"}') // I changed the incomplete JSON + stream.end() +}) + test('#112 end is not emmitted after error', function (t) { var ended = 0 var stream = JSONStream