Skip to content

repl: avoid crash when typing incomplete import statement#63573

Open
awbx wants to merge 2 commits into
nodejs:mainfrom
awbx:fix-repl-import-no-crash
Open

repl: avoid crash when typing incomplete import statement#63573
awbx wants to merge 2 commits into
nodejs:mainfrom
awbx:fix-repl-import-no-crash

Conversation

@awbx
Copy link
Copy Markdown

@awbx awbx commented May 25, 2026

Summary

Fixes #63551. Typing import (or any incomplete import form) at the Node.js REPL terminates the process with exit code 1 since v25.x, when the dynamic-import-suggestion helper was introduced. v22.x is unaffected.

Reproduction (broken on v25.x and v26.x, exits with status 1)

$ printf 'import\n.exit\n' | node --interactive
Welcome to Node.js v26.1.0.
Type ".help" for more information.
> node:internal/deps/acorn/acorn/dist/acorn:3768
    throw err
    ^

SyntaxError: Unexpected token (1:6)
    at pp$4.raise (node:internal/deps/acorn/acorn/dist/acorn:3766:15)
    ...

Root cause

  1. User types import\n.
  2. V8 throws SyntaxError("Cannot use import statement outside a module").
  3. REPL's error formatter detects that message and calls toDynamicImport(line) to build a "use dynamic import: …" suggestion.
  4. toDynamicImport calls acornParse(line, { sourceType: 'module' }) with no try/catch. For incomplete input acorn throws.
  5. The exception escapes through the error formatter, through the input evaluator, lands in lib/internal/readline/emitKeypressEvents.js, gets caught and rethrown (line 74).
  6. Process exits with code 1.

Fix

Two small changes in lib/repl.js:

  • toDynamicImport wraps the acornParse call in try/catch and returns an empty string on parse failure.
  • The error formatter calls toDynamicImport once, stores the result, and only appends the "alternatively use dynamic import: …" suffix when a non-empty string came back. When the line cannot be parsed (e.g. import alone), it falls back to a plain 'Cannot use import statement inside the Node.js REPL.' message — same intent, no crash.

When toDynamicImport succeeds, the produced message is byte-identical to the previous version, so the existing assertions in test/parallel/test-repl.js (lines 865–935, covering complete import forms) continue to pass without modification.

Tests

Added test/parallel/test-repl-import-statement-no-crash.js using the in-process startNewREPLServer helper (the same pattern as test-repl-null.js). The test emits an incomplete import line through the REPL and asserts:

  1. The line-handler does not throw out (i.e. the process stays alive).
  2. The REPL still surfaces a SyntaxError and the user-facing message Cannot use import statement inside the Node.js REPL on the output stream.

Verified to catch the regression: against unpatched v26.1.0 the test exits with code 1 (acorn SyntaxError escapes); against this branch's build the test exits 0.

Validated locally on Apple Silicon Mac (Node built from this branch):

  • out/Release/node test/parallel/test-repl-import-statement-no-crash.js → passes
  • out/Release/node test/parallel/test-repl.js → passes (the existing dynamic-import-suggestion assertions are unchanged)

Manual sanity check after build:

$ printf 'import\n.exit\n' | out/Release/node --interactive
Welcome to Node.js v27.0.0-pre.
Type ".help" for more information.
> import
^^^^^^

Uncaught SyntaxError: Cannot use import statement inside the Node.js REPL.

$ printf 'import x from "y"\n.exit\n' | out/Release/node --interactive
Welcome to Node.js v27.0.0-pre.
Type ".help" for more information.
> import x from "y"
^^^^^^

Uncaught:
SyntaxError: Cannot use import statement inside the Node.js REPL, alternatively use dynamic import: const { default: x } = await import("y");

Note on overlap with #63552

I noticed #63552 after submitting this — sorry for the duplicated effort. The root-cause analysis is the same and the core fix shape is functionally equivalent. The differences here are:

  1. Fallback message wording when parse fails. This PR returns '' from toDynamicImport on failure and updates the callsite to emit 'Cannot use import statement inside the Node.js REPL.' (no suggestion). repl: fix crash when bare 'import' is typed #63552 returns codeLine on failure, which makes the message read '… alternatively use dynamic import: import'. Minor UX preference.
  2. Test pattern. This PR uses the in-process startNewREPLServer helper (the change @aduh95 requested on repl: fix crash when bare 'import' is typed #63552's spawn-based test).

Happy to defer to #63552 if a maintainer prefers — closing this is one click.

Fixes: #63551

Typing `import` alone in the REPL triggers V8's "Cannot use import
statement outside a module" SyntaxError, which the REPL was enriching
with a dynamic-import suggestion via `toDynamicImport`. That helper
called acorn to re-parse the line; on incomplete input acorn throws,
the exception escaped the keypress handler, and the process exited
with code 1.

Catch the parse error inside `toDynamicImport` and fall back to a
plain error message when the input cannot be parsed as a complete
import statement.

Fixes: nodejs#63551
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. repl Issues and PRs related to the REPL subsystem. labels May 25, 2026
Replace the child_process.spawn-based regression test with the
in-process startNewREPLServer pattern (similar to test-repl-null.js).
This is faster, avoids subprocess flake, and lets the test assert on
the REPL output stream directly instead of bytes piped through stdio.

Verified to still catch the regression: the test exits with code 1
against unpatched v26.1.0 (acorn SyntaxError escapes) and exits 0
against the fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. repl Issues and PRs related to the REPL subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Using 'import' crashes the REPL

2 participants