Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

REPL is evaluating new lines before finishing execution of previous ones #39387

Open
ejose19 opened this issue Jul 14, 2021 · 3 comments · May be fixed by #39392
Open

REPL is evaluating new lines before finishing execution of previous ones #39387

ejose19 opened this issue Jul 14, 2021 · 3 comments · May be fixed by #39392
Labels
repl Issues and PRs related to the REPL subsystem.

Comments

@ejose19
Copy link
Contributor

ejose19 commented Jul 14, 2021

Version

16.4.2

Platform

Archlinux x86_64

Subsystem

No response

What steps will reproduce the bug?

run with: node --experimental-repl-await file.mjs

import { PassThrough } from 'stream';
import { start } from 'repl';

async function main() {
  const { input, output } = start({
    input: new PassThrough(),
    output: new PassThrough(),
    useGlobal: false,
  });

  const script = `const x = await new Promise((r) => setTimeout(() => r(1), 500));\nx;`;

  input.write(script);
  input.end();
  await new Promise((r) => setTimeout(r, 1000));
  output.end();

  let res = '';
  for await (const chunk of output) {
    res += chunk;
  }

  console.log(res);
}

main();

How often does it reproduce? Is there a required condition?

100%

What is the expected behavior?

New lines should be queued and processed only after previous lines have finished.

What do you see instead?

New lines are executed before previous lines have finished

Additional information

Debug output:

$ NODE_DEBUG='repl' node --experimental-repl-await file.mjs
REPL 14900: line "const x = await new Promise((r) => setTimeout(() => r(1), 500));"
REPL 14900: eval "const x = await new Promise((r) => setTimeout(() => r(1), 500));\n"
REPL 14900: line "x;"
REPL 14900: eval "x;\n"
REPL 14900: not recoverable, send to domain
REPL 14900: domain error
REPL 14900: finish null undefined
> Uncaught ReferenceError: x is not defined
> undefined
>

Sync code is not affected, but I would consider that a side effect of the sync code blocking the event loop preventing new lines being processed rather than a proper pause.

There's: de848ac that used another pause logic, but got reverted due to broking multiline repl.

This issue shouldn't be a blocker for #34733, as pause logic should be correct without depending on the event loop being blocked.

@guybedford
Copy link
Contributor

@ejose19 do you have any suggestions re a possible fix for this?

@ejose19
Copy link
Contributor Author

ejose19 commented Jul 14, 2021

Also, it would be better if pause logic is handled outside eval (can unpause when executing eval callback), so consumers providing their own eval function get this working "transparently".

@ejose19
Copy link
Contributor Author

ejose19 commented Jul 14, 2021

I've did some tests for await (let cmd of self) {, it worked for this particular issue but error messages were totally different and some tests were failing. I've also checked readline for any API that could be useful, and the closest was https://nodejs.org/api/readline.html#readline_rl_pause, but as stated in the description, there's no guarantee that it will immediately pause other events. No other ideas rn, but ideally finish should be in charge of executing or allowing execution of the next line, so when eval executes the callback, the next line can be safely processed.

@ejose19 ejose19 linked a pull request Jul 14, 2021 that will close this issue
@Ayase-252 Ayase-252 added the repl Issues and PRs related to the REPL subsystem. label Jul 15, 2021
@RedYetiDev RedYetiDev mentioned this issue Jun 24, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants