Skip to content

Commit aeefc8b

Browse files
gribnoysupgribnoysup
authored
fix(cli-repl): keep the raw mode off for the whole duration of the eval MONGOSH-1433 (#1460)
1 parent d8f287a commit aeefc8b

File tree

4 files changed

+119
-39
lines changed

4 files changed

+119
-39
lines changed

‎packages/cli-repl/src/async-repl.spec.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,21 @@ describe('AsyncRepl', () => {
102102
await expectInStream(output, 'meow');
103103
});
104104

105+
it('disables raw mode for input during both sync and async evaluation when async sigint is enabled', async() => {
106+
const { input, output, repl } = createDefaultAsyncRepl({ onAsyncSigint: () => false });
107+
let isRaw = true;
108+
Object.defineProperty(input, 'isRaw', {
109+
get() { return isRaw; },
110+
enumerable: true
111+
});
112+
(input as any).setRawMode = (value: boolean) => { isRaw = value; };
113+
repl.context.isRawMode = () => isRaw;
114+
115+
input.write('const before = isRawMode(); new Promise(setImmediate).then(() => ({before, after: isRawMode()}))\n');
116+
await expectInStream(output, 'before: false, after: false');
117+
expect(isRaw).to.equal(true);
118+
});
119+
105120
it('handles asynchronous exceptions well', async() => {
106121
const { input, output } = createDefaultAsyncRepl();
107122

‎packages/cli-repl/src/async-repl.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/* eslint-disable chai-friendly/no-unused-expressions */
22
import { Domain } from 'domain';
33
import type { EventEmitter } from 'events';
4+
import type { ReadStream } from 'tty';
45
import isRecoverableError from 'is-recoverable-error';
56
import { Interface, ReadLineOptions } from 'readline';
67
import type { ReplOptions, REPLServer } from 'repl';
@@ -75,11 +76,31 @@ export function start(opts: AsyncREPLOptions): REPLServer {
7576
repl.input,
7677
wrapNoSyncDomainError(repl.eval.bind(repl))));
7778

79+
const setRawMode = (mode: boolean): boolean => {
80+
const input = repl.input as ReadStream;
81+
const wasInRawMode = input.isRaw;
82+
if (typeof input.setRawMode === 'function') {
83+
input.setRawMode(mode);
84+
}
85+
return wasInRawMode;
86+
};
87+
7888
(repl as Mutable<typeof repl>).eval = async(
7989
input: string,
8090
context: any,
8191
filename: string,
8292
callback: (err: Error|null, result?: any) => void): Promise<void> => {
93+
let previouslyInRawMode;
94+
95+
if (onAsyncSigint) {
96+
// Unset raw mode during evaluation so that Ctrl+C raises a signal. This
97+
// is something REPL already does while originalEval is running, but as
98+
// the actual eval result might be a promise that we will be awaiting, we
99+
// want the raw mode to be disabled for the whole duration of our custom
100+
// async eval
101+
previouslyInRawMode = setRawMode(false);
102+
}
103+
83104
let result;
84105
repl.emit(evalStart, { input } as EvalStartEvent);
85106

@@ -150,6 +171,11 @@ export function start(opts: AsyncREPLOptions): REPLServer {
150171
evalResult.then(resolve, reject);
151172
});
152173
} finally {
174+
// Restore raw mode
175+
if (typeof previouslyInRawMode !== 'undefined') {
176+
setRawMode(previouslyInRawMode);
177+
}
178+
153179
// Remove our 'SIGINT' listener and re-install the REPL one(s).
154180
if (sigintListener !== undefined) {
155181
repl.removeListener('SIGINT', sigintListener);

‎packages/cli-repl/test/e2e.spec.ts‎

Lines changed: 73 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -705,47 +705,81 @@ describe('e2e', function() {
705705
}
706706
});
707707

708-
let shell: TestShell;
709-
beforeEach(async() => {
710-
shell = TestShell.start({ args: [ '--nodb' ], removeSigintListeners: true });
711-
await shell.waitForPrompt();
712-
shell.assertNoErrors();
708+
describe('non-interactive', function() {
709+
it('interrupts file execution', async function() {
710+
const filename = path.resolve(
711+
__dirname,
712+
'fixtures',
713+
'load',
714+
'long-sleep.js'
715+
);
716+
const shell = TestShell.start({
717+
args: ['--nodb', filename],
718+
removeSigintListeners: true,
719+
forceTerminal: true
720+
});
721+
722+
await eventually(() => {
723+
if (shell.output.includes('Long sleep')) {
724+
return;
725+
}
726+
throw new Error('Waiting for the file to load...');
727+
});
728+
729+
shell.kill('SIGINT');
730+
731+
await eventually(() => {
732+
if (shell.output.includes('MongoshInterruptedError')) {
733+
return;
734+
}
735+
throw new Error('Waiting for the interruption...');
736+
});
737+
});
713738
});
714739

715-
it('interrupts sync execution', async() => {
716-
await shell.executeLine('void process.removeAllListeners("SIGINT")');
717-
const result = shell.executeLine('while(true);');
718-
setTimeout(() => shell.kill('SIGINT'), 1000);
719-
await result;
720-
shell.assertContainsError('interrupted');
721-
});
722-
it('interrupts async awaiting', async() => {
723-
const result = shell.executeLine('new Promise(() => {});');
724-
setTimeout(() => shell.kill('SIGINT'), 3000);
725-
await result;
726-
shell.assertContainsOutput('Stopping execution...');
727-
});
728-
it('interrupts load()', async() => {
729-
const filename = path.resolve(__dirname, 'fixtures', 'load', 'infinite-loop.js');
730-
const result = shell.executeLine(`load(${JSON.stringify(filename)})`);
731-
setTimeout(() => shell.kill('SIGINT'), 3000);
732-
await result;
733-
// The while loop in the script is run as "sync" code
734-
shell.assertContainsError('interrupted');
735-
});
736-
it('behaves normally after an exception', async() => {
737-
await shell.executeLine('throw new Error()');
738-
await new Promise((resolve) => setTimeout(resolve, 100));
739-
shell.kill('SIGINT');
740-
await shell.waitForPrompt();
741-
await new Promise((resolve) => setTimeout(resolve, 100));
742-
shell.assertNotContainsOutput('interrupted');
743-
shell.assertNotContainsOutput('Stopping execution');
744-
});
745-
it('does not trigger MaxListenersExceededWarning', async() => {
746-
await shell.executeLine('for (let i = 0; i < 11; i++) { console.log("hi"); }\n');
747-
await shell.executeLine('for (let i = 0; i < 20; i++) (async() => { await sleep(0) })()');
748-
shell.assertNotContainsOutput('MaxListenersExceededWarning');
740+
describe('interactive', function() {
741+
let shell: TestShell;
742+
beforeEach(async() => {
743+
shell = TestShell.start({ args: [ '--nodb' ], removeSigintListeners: true });
744+
await shell.waitForPrompt();
745+
shell.assertNoErrors();
746+
});
747+
748+
it('interrupts sync execution', async() => {
749+
await shell.executeLine('void process.removeAllListeners("SIGINT")');
750+
const result = shell.executeLine('while(true);');
751+
setTimeout(() => shell.kill('SIGINT'), 1000);
752+
await result;
753+
shell.assertContainsError('interrupted');
754+
});
755+
it('interrupts async awaiting', async() => {
756+
const result = shell.executeLine('new Promise(() => {});');
757+
setTimeout(() => shell.kill('SIGINT'), 3000);
758+
await result;
759+
shell.assertContainsOutput('Stopping execution...');
760+
});
761+
it('interrupts load()', async() => {
762+
const filename = path.resolve(__dirname, 'fixtures', 'load', 'infinite-loop.js');
763+
const result = shell.executeLine(`load(${JSON.stringify(filename)})`);
764+
setTimeout(() => shell.kill('SIGINT'), 3000);
765+
await result;
766+
// The while loop in the script is run as "sync" code
767+
shell.assertContainsError('interrupted');
768+
});
769+
it('behaves normally after an exception', async() => {
770+
await shell.executeLine('throw new Error()');
771+
await new Promise((resolve) => setTimeout(resolve, 100));
772+
shell.kill('SIGINT');
773+
await shell.waitForPrompt();
774+
await new Promise((resolve) => setTimeout(resolve, 100));
775+
shell.assertNotContainsOutput('interrupted');
776+
shell.assertNotContainsOutput('Stopping execution');
777+
});
778+
it('does not trigger MaxListenersExceededWarning', async() => {
779+
await shell.executeLine('for (let i = 0; i < 11; i++) { console.log("hi"); }\n');
780+
await shell.executeLine('for (let i = 0; i < 20; i++) (async() => { await sleep(0) })()');
781+
shell.assertNotContainsOutput('MaxListenersExceededWarning');
782+
});
749783
});
750784
});
751785

‎packages/cli-repl/test/fixtures/load/long-sleep.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
/* eslint-disable */
2+
console.log('Long sleep');
3+
(async() => {
4+
await sleep(1_000_000);
5+
})();

0 commit comments

Comments
 (0)