From f4beb25cfebde2357e51483d93259ad850c7073e Mon Sep 17 00:00:00 2001
From: Robert Craigie <robert@craigie.dev>
Date: Sat, 21 Sep 2024 21:56:44 +0100
Subject: [PATCH] feat(cli): mention nodejs extra if nodeenv install fails

---
 pyright/node.py    | 15 ++++++++++++---
 tests/test_node.py | 22 ++++++++++++++++++++++
 2 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/pyright/node.py b/pyright/node.py
index 294b0ba..0c3690e 100644
--- a/pyright/node.py
+++ b/pyright/node.py
@@ -44,8 +44,7 @@ def _postfix_for_target(target: Target) -> str:
 def _ensure_node_env(target: Target) -> Path:
     log.debug('Checking for nodeenv %s binary', target)
 
-    path = BINARIES_DIR.joinpath(target + _postfix_for_target(target))
-
+    path = _get_nodeenv_path(target)
     log.debug('Using %s path for binary', path)
 
     if path.exists() and not NODE_VERSION:
@@ -59,6 +58,10 @@ def _ensure_node_env(target: Target) -> Path:
     return path
 
 
+def _get_nodeenv_path(target: Target) -> Path:
+    return BINARIES_DIR.joinpath(target + _postfix_for_target(target))
+
+
 def _get_global_binary(target: Target) -> Optional[Path]:
     log.debug('Checking for global target binary: %s', target)
 
@@ -85,7 +88,13 @@ def _install_node_env() -> None:
         args += ['--node', NODE_VERSION, '--force']
     args.append(str(ENV_DIR))
     log.debug('Running command with args: %s', args)
-    subprocess.run(args, check=True)
+
+    try:
+        subprocess.run(args, check=True)
+    except subprocess.CalledProcessError as exc:
+        raise RuntimeError(
+            'nodeenv failed; for more reliable node.js binaries try `pip install pyright[nodejs]`'
+        ) from exc
 
 
 class GlobalStrategy(NamedTuple):
diff --git a/tests/test_node.py b/tests/test_node.py
index cf466ed..06d9862 100644
--- a/tests/test_node.py
+++ b/tests/test_node.py
@@ -1,4 +1,5 @@
 import os
+import sys
 import subprocess
 from typing import TYPE_CHECKING, Tuple
 from pathlib import Path
@@ -90,6 +91,27 @@ def test_node_version_env() -> None:
     assert maybe_decode(proc.stdout).strip() == 'v13.1.0'
 
 
+@mock.patch('pyright.node.USE_GLOBAL_NODE', False)
+@mock.patch('pyright.node.USE_NODEJS_WHEEL', False)
+@mock.patch('pyright.node.NODE_VERSION', None)
+@mock.patch('pyright.node.BINARIES_DIR', pyright.node.BINARIES_DIR.joinpath('empty'))
+def test_nodeenv_flaky_error(fake_process: FakeProcess) -> None:
+    """A helpful error is raised when nodeenv fails."""
+    fake_process.register_subprocess(  # pyright: ignore[reportUnknownMemberType]
+        [sys.executable, '-m', 'nodeenv', str(pyright.node.ENV_DIR)],
+        stdout='this thing is flaky',
+        returncode=1,
+    )
+
+    with pytest.raises(RuntimeError, match=r'install pyright\[nodejs\]'):
+        pyright.node.run(
+            'node',
+            '--version',
+            stdout=subprocess.PIPE,
+            stderr=subprocess.STDOUT,
+        )
+
+
 def test_update_path_env(tmp_path: Path) -> None:
     """The _update_path_env() function correctly appends the target binary path to the PATH environment variable"""
     target = tmp_path / 'bin'