From 32ba8aea0b8c43b63c820ae36671a6ba20361273 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 17 Jun 2017 02:51:23 +0200 Subject: [PATCH] repl: fix old history error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backport-PR-URL: https://github.com/nodejs/node/pull/14392 Backport-Reviewed-By: James M Snell PR-URL: https://github.com/nodejs/node/pull/13733 Reviewed-By: Luigi Pinca Reviewed-By: James M Snell Reviewed-By: Colin Ihrig Reviewed-By: Tobias Nießen --- lib/internal/repl.js | 64 +++++++++++-------- .../old-repl-history-file-faulty.json | 1 + test/fixtures/old-repl-history-file-obj.json | 4 ++ test/parallel/test-repl-persistent-history.js | 27 ++++++-- 4 files changed, 67 insertions(+), 29 deletions(-) create mode 100644 test/fixtures/old-repl-history-file-faulty.json create mode 100644 test/fixtures/old-repl-history-file-obj.json diff --git a/lib/internal/repl.js b/lib/internal/repl.js index 4c27fa2746390f..874bfd6fbbd183 100644 --- a/lib/internal/repl.js +++ b/lib/internal/repl.js @@ -15,6 +15,11 @@ module.exports.createInternalRepl = createRepl; // The debounce is to guard against code pasted into the REPL. const kDebounceHistoryMS = 15; +function _writeToOutput(repl, message) { + repl._writeToOutput(message); + repl._refreshLine(); +} + function createRepl(env, opts, cb) { if (typeof opts === 'function') { cb = opts; @@ -80,9 +85,8 @@ function setupHistory(repl, historyPath, oldHistoryPath, ready) { try { historyPath = path.join(os.homedir(), '.node_repl_history'); } catch (err) { - repl._writeToOutput('\nError: Could not get the home directory.\n' + - 'REPL session history will not be persisted.\n'); - repl._refreshLine(); + _writeToOutput(repl, '\nError: Could not get the home directory.\n' + + 'REPL session history will not be persisted.\n'); debug(err.stack); repl._historyPrev = _replHistoryMessage; @@ -103,9 +107,8 @@ function setupHistory(repl, historyPath, oldHistoryPath, ready) { if (err) { // Cannot open history file. // Don't crash, just don't persist history. - repl._writeToOutput('\nError: Could not open history file.\n' + - 'REPL session history will not be persisted.\n'); - repl._refreshLine(); + _writeToOutput(repl, '\nError: Could not open history file.\n' + + 'REPL session history will not be persisted.\n'); debug(err.stack); repl._historyPrev = _replHistoryMessage; @@ -132,18 +135,13 @@ function setupHistory(repl, historyPath, oldHistoryPath, ready) { } else if (oldHistoryPath === historyPath) { // If pre-v3.0, the user had set NODE_REPL_HISTORY_FILE to // ~/.node_repl_history, warn the user about it and proceed. - repl._writeToOutput( - '\nThe old repl history file has the same name and location as ' + + _writeToOutput( + repl, + '\nThe old repl history file has the same name and location as ' + `the new one i.e., ${historyPath} and is empty.\nUsing it as is.\n`); - repl._refreshLine(); } else if (oldHistoryPath) { - // Grab data from the older pre-v3.0 JSON NODE_REPL_HISTORY_FILE format. - repl._writeToOutput( - '\nConverting old JSON repl history to line-separated history.\n' + - `The new repl history file can be found at ${historyPath}.\n`); - repl._refreshLine(); - + let threw = false; try { // Pre-v3.0, repl history was stored as JSON. // Try and convert it to line separated history. @@ -152,15 +150,31 @@ function setupHistory(repl, historyPath, oldHistoryPath, ready) { // Only attempt to use the history if there was any. if (oldReplJSONHistory) repl.history = JSON.parse(oldReplJSONHistory); - if (!Array.isArray(repl.history)) { - throw new Error('Expected array, got ' + typeof repl.history); + if (Array.isArray(repl.history)) { + repl.history = repl.history.slice(0, repl.historySize); + } else { + threw = true; + _writeToOutput( + repl, + '\nError: The old history file data has to be an Array.\n' + + 'REPL session history will not be persisted.\n'); } - repl.history = repl.history.slice(0, repl.historySize); } catch (err) { - if (err.code !== 'ENOENT') { - return ready( - new Error(`Could not parse history data in ${oldHistoryPath}.`)); - } + // Cannot open or parse history file. + // Don't crash, just don't persist history. + threw = true; + const type = err instanceof SyntaxError ? 'parse' : 'open'; + _writeToOutput(repl, `\nError: Could not ${type} old history file.\n` + + 'REPL session history will not be persisted.\n'); + } + if (!threw) { + // Grab data from the older pre-v3.0 JSON NODE_REPL_HISTORY_FILE format. + _writeToOutput( + repl, + '\nConverted old JSON repl history to line-separated history.\n' + + `The new repl history file can be found at ${historyPath}.\n`); + } else { + repl.history = []; } } @@ -223,12 +237,12 @@ function setupHistory(repl, historyPath, oldHistoryPath, ready) { function _replHistoryMessage() { if (this.history.length === 0) { - this._writeToOutput( - '\nPersistent history support disabled. ' + + _writeToOutput( + this, + '\nPersistent history support disabled. ' + 'Set the NODE_REPL_HISTORY environment\nvariable to ' + 'a valid, user-writable path to enable.\n' ); - this._refreshLine(); } this._historyPrev = Interface.prototype._historyPrev; return this._historyPrev(); diff --git a/test/fixtures/old-repl-history-file-faulty.json b/test/fixtures/old-repl-history-file-faulty.json new file mode 100644 index 00000000000000..417b7b5370df81 --- /dev/null +++ b/test/fixtures/old-repl-history-file-faulty.json @@ -0,0 +1 @@ +undefined diff --git a/test/fixtures/old-repl-history-file-obj.json b/test/fixtures/old-repl-history-file-obj.json new file mode 100644 index 00000000000000..43160121b8f9e1 --- /dev/null +++ b/test/fixtures/old-repl-history-file-obj.json @@ -0,0 +1,4 @@ +{ + "a": "'=^.^='", + "b": "'hello world'" +} diff --git a/test/parallel/test-repl-persistent-history.js b/test/parallel/test-repl-persistent-history.js index fe219d716ebf45..8b47cadf11d196 100644 --- a/test/parallel/test-repl-persistent-history.js +++ b/test/parallel/test-repl-persistent-history.js @@ -56,13 +56,19 @@ const prompt = '> '; const replDisabled = '\nPersistent history support disabled. Set the ' + 'NODE_REPL_HISTORY environment\nvariable to a valid, ' + 'user-writable path to enable.\n'; -const convertMsg = '\nConverting old JSON repl history to line-separated ' + +const convertMsg = '\nConverted old JSON repl history to line-separated ' + 'history.\nThe new repl history file can be found at ' + `${path.join(common.tmpDir, '.node_repl_history')}.\n`; const homedirErr = '\nError: Could not get the home directory.\n' + 'REPL session history will not be persisted.\n'; const replFailedRead = '\nError: Could not open history file.\n' + 'REPL session history will not be persisted.\n'; +const oldHistoryFailedOpen = '\nError: Could not open old history file.\n' + + 'REPL session history will not be persisted.\n'; +const oldHistoryFailedParse = '\nError: Could not parse old history file.\n' + + 'REPL session history will not be persisted.\n'; +const oldHistoryObj = '\nError: The old history file data has to be an Array' + + '.\nREPL session history will not be persisted.\n'; const sameHistoryFilePaths = '\nThe old repl history file has the same name ' + 'and location as the new one i.e., ' + path.join(common.tmpDir, '.node_repl_history') + @@ -72,6 +78,10 @@ const fixtures = common.fixturesDir; const historyFixturePath = path.join(fixtures, '.node_repl_history'); const historyPath = path.join(common.tmpDir, '.fixture_copy_repl_history'); const historyPathFail = path.join(common.tmpDir, '.node_repl\u0000_history'); +const oldHistoryPathObj = path.join(fixtures, + 'old-repl-history-file-obj.json'); +const oldHistoryPathFaulty = path.join(fixtures, + 'old-repl-history-file-faulty.json'); const oldHistoryPath = path.join(fixtures, 'old-repl-history-file.json'); const enoentHistoryPath = path.join(fixtures, 'enoent-repl-history-file.json'); const emptyHistoryPath = path.join(fixtures, '.empty-repl-history-file'); @@ -93,10 +103,19 @@ const tests = [ expected: [prompt, replDisabled, prompt] }, { - env: { NODE_REPL_HISTORY: '', - NODE_REPL_HISTORY_FILE: enoentHistoryPath }, + env: { NODE_REPL_HISTORY_FILE: enoentHistoryPath }, test: [UP], - expected: [prompt, replDisabled, prompt] + expected: [prompt, oldHistoryFailedOpen, prompt] + }, + { + env: { NODE_REPL_HISTORY_FILE: oldHistoryPathObj }, + test: [UP], + expected: [prompt, oldHistoryObj, prompt] + }, + { + env: { NODE_REPL_HISTORY_FILE: oldHistoryPathFaulty }, + test: [UP], + expected: [prompt, oldHistoryFailedParse, prompt] }, { env: { NODE_REPL_HISTORY: '',