Skip to content

Commit 24b384d

Browse files
BridgeARaddaleax
BridgeAR
authored and
addaleax
committed
repl: fix old history error handling
Backport-PR-URL: #14392 Backport-Reviewed-By: James M Snell <[email protected]> PR-URL: #13733 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
1 parent 6fce1a3 commit 24b384d

File tree

4 files changed

+67
-29
lines changed

4 files changed

+67
-29
lines changed

‎lib/internal/repl.js‎

Lines changed: 39 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@ module.exports.createInternalRepl = createRepl;
1515
// The debounce is to guard against code pasted into the REPL.
1616
const kDebounceHistoryMS = 15;
1717

18+
function _writeToOutput(repl, message) {
19+
repl._writeToOutput(message);
20+
repl._refreshLine();
21+
}
22+
1823
function createRepl(env, opts, cb) {
1924
if (typeof opts === 'function') {
2025
cb = opts;
@@ -80,9 +85,8 @@ function setupHistory(repl, historyPath, oldHistoryPath, ready) {
8085
try {
8186
historyPath = path.join(os.homedir(), '.node_repl_history');
8287
} catch (err) {
83-
repl._writeToOutput('\nError: Could not get the home directory.\n' +
84-
'REPL session history will not be persisted.\n');
85-
repl._refreshLine();
88+
_writeToOutput(repl, '\nError: Could not get the home directory.\n' +
89+
'REPL session history will not be persisted.\n');
8690

8791
debug(err.stack);
8892
repl._historyPrev = _replHistoryMessage;
@@ -103,9 +107,8 @@ function setupHistory(repl, historyPath, oldHistoryPath, ready) {
103107
if (err) {
104108
// Cannot open history file.
105109
// Don't crash, just don't persist history.
106-
repl._writeToOutput('\nError: Could not open history file.\n' +
107-
'REPL session history will not be persisted.\n');
108-
repl._refreshLine();
110+
_writeToOutput(repl, '\nError: Could not open history file.\n' +
111+
'REPL session history will not be persisted.\n');
109112
debug(err.stack);
110113

111114
repl._historyPrev = _replHistoryMessage;
@@ -132,18 +135,13 @@ function setupHistory(repl, historyPath, oldHistoryPath, ready) {
132135
} else if (oldHistoryPath === historyPath) {
133136
// If pre-v3.0, the user had set NODE_REPL_HISTORY_FILE to
134137
// ~/.node_repl_history, warn the user about it and proceed.
135-
repl._writeToOutput(
136-
'\nThe old repl history file has the same name and location as ' +
138+
_writeToOutput(
139+
repl,
140+
'\nThe old repl history file has the same name and location as ' +
137141
`the new one i.e., ${historyPath} and is empty.\nUsing it as is.\n`);
138-
repl._refreshLine();
139142

140143
} else if (oldHistoryPath) {
141-
// Grab data from the older pre-v3.0 JSON NODE_REPL_HISTORY_FILE format.
142-
repl._writeToOutput(
143-
'\nConverting old JSON repl history to line-separated history.\n' +
144-
`The new repl history file can be found at ${historyPath}.\n`);
145-
repl._refreshLine();
146-
144+
let threw = false;
147145
try {
148146
// Pre-v3.0, repl history was stored as JSON.
149147
// Try and convert it to line separated history.
@@ -152,15 +150,31 @@ function setupHistory(repl, historyPath, oldHistoryPath, ready) {
152150
// Only attempt to use the history if there was any.
153151
if (oldReplJSONHistory) repl.history = JSON.parse(oldReplJSONHistory);
154152

155-
if (!Array.isArray(repl.history)) {
156-
throw new Error('Expected array, got ' + typeof repl.history);
153+
if (Array.isArray(repl.history)) {
154+
repl.history = repl.history.slice(0, repl.historySize);
155+
} else {
156+
threw = true;
157+
_writeToOutput(
158+
repl,
159+
'\nError: The old history file data has to be an Array.\n' +
160+
'REPL session history will not be persisted.\n');
157161
}
158-
repl.history = repl.history.slice(0, repl.historySize);
159162
} catch (err) {
160-
if (err.code !== 'ENOENT') {
161-
return ready(
162-
new Error(`Could not parse history data in ${oldHistoryPath}.`));
163-
}
163+
// Cannot open or parse history file.
164+
// Don't crash, just don't persist history.
165+
threw = true;
166+
const type = err instanceof SyntaxError ? 'parse' : 'open';
167+
_writeToOutput(repl, `\nError: Could not ${type} old history file.\n` +
168+
'REPL session history will not be persisted.\n');
169+
}
170+
if (!threw) {
171+
// Grab data from the older pre-v3.0 JSON NODE_REPL_HISTORY_FILE format.
172+
_writeToOutput(
173+
repl,
174+
'\nConverted old JSON repl history to line-separated history.\n' +
175+
`The new repl history file can be found at ${historyPath}.\n`);
176+
} else {
177+
repl.history = [];
164178
}
165179
}
166180

@@ -223,12 +237,12 @@ function setupHistory(repl, historyPath, oldHistoryPath, ready) {
223237

224238
function _replHistoryMessage() {
225239
if (this.history.length === 0) {
226-
this._writeToOutput(
227-
'\nPersistent history support disabled. ' +
240+
_writeToOutput(
241+
this,
242+
'\nPersistent history support disabled. ' +
228243
'Set the NODE_REPL_HISTORY environment\nvariable to ' +
229244
'a valid, user-writable path to enable.\n'
230245
);
231-
this._refreshLine();
232246
}
233247
this._historyPrev = Interface.prototype._historyPrev;
234248
return this._historyPrev();

‎test/fixtures/old-repl-history-file-faulty.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
undefined

‎test/fixtures/old-repl-history-file-obj.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"a": "'=^.^='",
3+
"b": "'hello world'"
4+
}

‎test/parallel/test-repl-persistent-history.js‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,19 @@ const prompt = '> ';
5656
const replDisabled = '\nPersistent history support disabled. Set the ' +
5757
'NODE_REPL_HISTORY environment\nvariable to a valid, ' +
5858
'user-writable path to enable.\n';
59-
const convertMsg = '\nConverting old JSON repl history to line-separated ' +
59+
const convertMsg = '\nConverted old JSON repl history to line-separated ' +
6060
'history.\nThe new repl history file can be found at ' +
6161
`${path.join(common.tmpDir, '.node_repl_history')}.\n`;
6262
const homedirErr = '\nError: Could not get the home directory.\n' +
6363
'REPL session history will not be persisted.\n';
6464
const replFailedRead = '\nError: Could not open history file.\n' +
6565
'REPL session history will not be persisted.\n';
66+
const oldHistoryFailedOpen = '\nError: Could not open old history file.\n' +
67+
'REPL session history will not be persisted.\n';
68+
const oldHistoryFailedParse = '\nError: Could not parse old history file.\n' +
69+
'REPL session history will not be persisted.\n';
70+
const oldHistoryObj = '\nError: The old history file data has to be an Array' +
71+
'.\nREPL session history will not be persisted.\n';
6672
const sameHistoryFilePaths = '\nThe old repl history file has the same name ' +
6773
'and location as the new one i.e., ' +
6874
path.join(common.tmpDir, '.node_repl_history') +
@@ -72,6 +78,10 @@ const fixtures = common.fixturesDir;
7278
const historyFixturePath = path.join(fixtures, '.node_repl_history');
7379
const historyPath = path.join(common.tmpDir, '.fixture_copy_repl_history');
7480
const historyPathFail = path.join(common.tmpDir, '.node_repl\u0000_history');
81+
const oldHistoryPathObj = path.join(fixtures,
82+
'old-repl-history-file-obj.json');
83+
const oldHistoryPathFaulty = path.join(fixtures,
84+
'old-repl-history-file-faulty.json');
7585
const oldHistoryPath = path.join(fixtures, 'old-repl-history-file.json');
7686
const enoentHistoryPath = path.join(fixtures, 'enoent-repl-history-file.json');
7787
const emptyHistoryPath = path.join(fixtures, '.empty-repl-history-file');
@@ -93,10 +103,19 @@ const tests = [
93103
expected: [prompt, replDisabled, prompt]
94104
},
95105
{
96-
env: { NODE_REPL_HISTORY: '',
97-
NODE_REPL_HISTORY_FILE: enoentHistoryPath },
106+
env: { NODE_REPL_HISTORY_FILE: enoentHistoryPath },
98107
test: [UP],
99-
expected: [prompt, replDisabled, prompt]
108+
expected: [prompt, oldHistoryFailedOpen, prompt]
109+
},
110+
{
111+
env: { NODE_REPL_HISTORY_FILE: oldHistoryPathObj },
112+
test: [UP],
113+
expected: [prompt, oldHistoryObj, prompt]
114+
},
115+
{
116+
env: { NODE_REPL_HISTORY_FILE: oldHistoryPathFaulty },
117+
test: [UP],
118+
expected: [prompt, oldHistoryFailedParse, prompt]
100119
},
101120
{
102121
env: { NODE_REPL_HISTORY: '',

0 commit comments

Comments
 (0)