Skip to content

repl: don't crash if cannot open history file #3630

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Nov 3, 2015
Merged

repl: don't crash if cannot open history file #3630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion lib/internal/repl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,16 @@ function setupHistory(repl, historyPath, oldHistoryPath, ready) {

function oninit(err, hnd) {
if (err) {
return ready(err);
// Cannot open history file.
// Don't crash, just don't persist history.
repl._writeToOutput('\nError: Could not open history file.\n' +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe "the history file"?

'REPL session history will not be persisted.\n');
repl._refreshLine();
debug(err.stack);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: space after here for readability

(Just visual separation between logging and closing logic)


repl._historyPrev = _replHistoryMessage;
repl.resume();
return ready(null, repl);
}
fs.close(hnd, onclose);
}
Expand Down
9 changes: 9 additions & 0 deletions test/sequential/test-repl-persistent-history.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,13 @@ const convertMsg = '\nConverting old JSON repl history to line-separated ' +
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';
// File paths
const fixtures = path.join(common.testDir, 'fixtures');
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');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just an invalid path instead?

What if we do have a invalid mode file then? What if the file is read-only?

I guess at that point we can just not care, since we make the file with specific perms and then it's probably at the hands of the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was more to test when fs.open fails, which is the same thing that will happen when we don't have permission to open the file in the first place.

const oldHistoryPath = path.join(fixtures, 'old-repl-history-file.json');
const enoentHistoryPath = path.join(fixtures, 'enoent-repl-history-file.json');
const defaultHistoryPath = path.join(common.tmpDir, '.node_repl_history');
Expand Down Expand Up @@ -147,6 +150,12 @@ const tests = [{
test: [UP, UP, UP, CLEAR],
expected: [prompt, convertMsg, prompt, prompt + '\'=^.^=\'', prompt]
},
{
env: { NODE_REPL_HISTORY: historyPathFail,
NODE_REPL_HISTORY_SIZE: 1 },
test: [UP],
expected: [prompt, replFailedRead, prompt, replDisabled, prompt]
},
{ // Make sure this is always the last test, since we change os.homedir()
before: function mockHomedirFailure() {
// Mock os.homedir() failure
Expand Down