From f2930716d0c743179708d6db0dfa76b5c1224d62 Mon Sep 17 00:00:00 2001 From: Varun Sharma Date: Tue, 19 Mar 2019 19:29:03 +0000 Subject: [PATCH] BugFix #636 Pause and resume (In a quick succession) gets you lot of exceptions and an infinite loop (#637) --- papaparse.js | 21 ++++++++++++++++----- tests/node-tests.js | 36 +++++++++++++++++++++++++++++++++++ tests/test-cases.js | 46 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 5 deletions(-) diff --git a/papaparse.js b/papaparse.js index 70c4edc8..6e68a528 100755 --- a/papaparse.js +++ b/papaparse.js @@ -473,6 +473,7 @@ License: MIT this._handle = null; this._finished = false; this._completed = false; + this._halted = false; this._input = null; this._baseIndex = 0; this._partialLine = ''; @@ -497,6 +498,7 @@ License: MIT chunk = modifiedChunk; } this.isFirstChunk = false; + this._halted = false; // Rejoin the line we likely just split in two by chunking the file var aggregate = this._partialLine + chunk; @@ -504,8 +506,10 @@ License: MIT var results = this._handle.parse(aggregate, this._baseIndex, !this._finished); - if (this._handle.paused() || this._handle.aborted()) + if (this._handle.paused() || this._handle.aborted()) { + this._halted = true; return; + } var lastIndex = results.meta.cursor; @@ -531,8 +535,10 @@ License: MIT else if (isFunction(this._config.chunk) && !isFakeChunk) { this._config.chunk(results, this._handle); - if (this._handle.paused() || this._handle.aborted()) + if (this._handle.paused() || this._handle.aborted()) { + this._halted = true; return; + } results = undefined; this._completeResults = undefined; } @@ -992,7 +998,6 @@ License: MIT // One goal is to minimize the use of regular expressions... var FLOAT = /^\s*-?(\d*\.?\d+|\d+\.?\d*)(e[-+]?\d+)?\s*$/i; var ISO_DATE = /(\d{4}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d:[0-5]\d\.\d+([+-][0-2]\d:[0-5]\d|Z))|(\d{4}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d:[0-5]\d([+-][0-2]\d:[0-5]\d|Z))|(\d{4}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d([+-][0-2]\d:[0-5]\d|Z))/; - var self = this; var _stepCounter = 0; // Number of times step was called (number of rows parsed) var _rowCounter = 0; // Number of rows that have been parsed so far @@ -1089,8 +1094,14 @@ License: MIT this.resume = function() { - _paused = false; - self.streamer.parseChunk(_input, true); + if(self.streamer._halted) { + _paused = false; + self.streamer.parseChunk(_input, true); + } else { + // Bugfix: #636 In case the processing hasn't halted yet + // wait for it to halt in order to resume + setTimeout(this.resume, 3); + } }; this.aborted = function() diff --git a/tests/node-tests.js b/tests/node-tests.js index fe671dfb..a067bfe7 100644 --- a/tests/node-tests.js +++ b/tests/node-tests.js @@ -41,6 +41,42 @@ describe('PapaParse', function() { assertLongSampleParsedCorrectly(Papa.parse(longSampleRawCsv)); }); + it('Pause and resume works (Regression Test for Bug #636)', function(done) { + this.timeout(30000); + var mod200Rows = [ + ["Etiam a dolor vitae est vestibulum","84","DEF"], + ["Etiam a dolor vitae est vestibulum","84","DEF"], + ["Lorem ipsum dolor sit","42","ABC"], + ["Etiam a dolor vitae est vestibulum","84","DEF"], + ["Etiam a dolor vitae est vestibulum","84"], + ["Lorem ipsum dolor sit","42","ABC"], + ["Etiam a dolor vitae est vestibulum","84","DEF"], + ["Etiam a dolor vitae est vestibulum","84","DEF"], + ["Lorem ipsum dolor sit","42","ABC"], + ["Lorem ipsum dolor sit","42"] + ]; + var stepped = 0; + var dataRows = []; + Papa.parse(fs.createReadStream(__dirname + '/verylong-sample.csv'), { + step: function(results, parser) { + stepped++; + if (results) + { + parser.pause(); + parser.resume(); + if (results.data && stepped % 200 === 0) { + dataRows.push(results.data); + } + } + }, + complete: function() { + assert.strictEqual(2001, stepped); + assert.deepEqual(mod200Rows, dataRows); + done(); + } + }); + }); + it('asynchronously parsed CSV should be correctly parsed', function(done) { Papa.parse(longSampleRawCsv, { complete: function(parsedCsv) { diff --git a/tests/test-cases.js b/tests/test-cases.js index 07b8a535..d9dd4594 100644 --- a/tests/test-cases.js +++ b/tests/test-cases.js @@ -1780,6 +1780,49 @@ describe('Unparse Tests', function() { var CUSTOM_TESTS = [ + { + description: "Pause and resume works (Regression Test for Bug #636)", + disabled: !XHR_ENABLED, + timeout: 30000, + expected: [2001, [ + ["Etiam a dolor vitae est vestibulum","84","DEF"], + ["Etiam a dolor vitae est vestibulum","84","DEF"], + ["Lorem ipsum dolor sit","42","ABC"], + ["Etiam a dolor vitae est vestibulum","84","DEF"], + ["Etiam a dolor vitae est vestibulum","84"], + ["Lorem ipsum dolor sit","42","ABC"], + ["Etiam a dolor vitae est vestibulum","84","DEF"], + ["Etiam a dolor vitae est vestibulum","84","DEF"], + ["Lorem ipsum dolor sit","42","ABC"], + ["Lorem ipsum dolor sit","42"] + ], 0], + run: function(callback) { + var stepped = 0; + var dataRows = []; + var errorCount = 0; + var output = []; + Papa.parse(BASE_PATH + "verylong-sample.csv", { + download: true, + step: function(results, parser) { + stepped++; + if (results) + { + parser.pause(); + parser.resume(); + if (results.data && stepped % 200 === 0) { + dataRows.push(results.data); + } + } + }, + complete: function() { + output.push(stepped); + output.push(dataRows); + output.push(errorCount); + callback(output); + } + }); + } + }, { description: "Complete is called with all results if neither step nor chunk is defined", expected: [['A', 'b', 'c'], ['d', 'E', 'f'], ['G', 'h', 'i']], @@ -2263,6 +2306,9 @@ var CUSTOM_TESTS = [ describe('Custom Tests', function() { function generateTest(test) { (test.disabled ? it.skip : it)(test.description, function(done) { + if(test.timeout) { + this.timeout(test.timeout); + } test.run(function(actual) { assert.deepEqual(JSON.stringify(actual), JSON.stringify(test.expected)); done();