fix(cli-repl): do not run prompt and initial input in parallel MONGOSH-1617 #1856
Conversation
…H-1617 Users were getting incorrect behavior when: 1. A custom asynchronous prompt was set outside of the main REPL input, e.g. in a `.mongoshrc.js` file or a `--eval` script, and 2. The main REPL input was readable before we started evaluating the prompt. because then, the first lines of input and the prompt may have ended up executing in parallel. Setting the prompt before starting to process input should resolve this issue.
| function nonnull<T>(value: T | null | undefined): NonNullable<T> { | ||
| if (!value) throw new Error(); | ||
| return value; | ||
| } |
There was a problem hiding this comment.
There are some drive-by fixes for type errors in the test files here, mostly things I missed in #1849 because we apply different TS configs for out test and source files
| }); | ||
| // The number of newlines here matters | ||
| shell.writeInput( | ||
| 'sleep(100);print([1,2,3,4,5,6,7,8,9,10].reduce(\n(a,b) => { return a*b; }, 1))\n\n\n\n', |
There was a problem hiding this comment.
probably silly question, but why is this sleeping longer if we want to make sure the prompt is evaluated first? also why does the number of newlines matter? 😅
There was a problem hiding this comment.
but why is this sleeping longer if we want to make sure the prompt is evaluated first?
I don't think it's a silly question, but I also don't quite know if I follow it – the sleep(100) is to make sure that the evaluation of this input takes longer than to evaluate any number of prompt() calls, that's the goal :)
also why does the number of newlines matter? 😅
Sooo ... admittedly, I only dug into this far enough to be fairly confident that I've identified the root cause, not the full behavior.
Without this fix, the prompt evaluation and the input evaluation would start in parallel. The REPL assumes that, if an evaluation finishes, it can consume the next line of input, and so it ends up reading more lines from the input than it should. The exact outcome was a bit flaky – the tests I've added to this file are this one for --nodb as a more isolated case that didn't give the right results, and the other one below that's a bit closer to the original reproduction from the ticket (but requires DB interaction and is even harder to debug because of that).
So I can only really say that the number of newlines matters because I've observed differences in behavior depending on the number newlines (e.g. mongosh printing the result of the expression or omitting it entirely), not because I fully understand how the number of newlines influences mongosh's behavior.
(I do know that the reason why the original "Mongosh not initialized yet" error shows up; namely, when the REPL keeps consuming lines from stdin, it will do so until it hits the end of the input. When it does, that makes it emit the exit event, which makes mongosh close its resources. And because we were consuming input more quickly than we should, this happened while we were still evaluating the user input, not afterwards.)
There was a problem hiding this comment.
oh I get it now, thank you for the explanation!
Users were getting incorrect behavior when:
.mongoshrc.jsfile or a--evalscript, andbecause then, the first lines of input and the prompt may have ended up executing in parallel. Setting the prompt before starting to process input should resolve this issue.