fix(command-dev): use execa to launch framework server (#2857)

Author: erezrokah

npm-shrinkwrap.json

@@ -183,7 +183,6 @@
     "update-notifier": "^5.0.0",
     "uuid": "^8.0.0",
     "wait-port": "^0.2.2",
-    "which": "^2.0.2",
     "winston": "^3.2.1",
     "write-file-atomic": "^3.0.0"
   },

package.json

@@ -1,15 +1,14 @@
-const childProcess = require('child_process')
 const path = require('path')
 const process = require('process')
 const { promisify } = require('util')
 
 const { flags: flagsLib } = require('@oclif/command')
 const boxen = require('boxen')
 const chalk = require('chalk')
+const execa = require('execa')
 const StaticServer = require('static-server')
 const stripAnsiCc = require('strip-ansi-control-characters')
 const waitPort = require('wait-port')
-const which = require('which')
 
 const { startFunctionsServer } = require('../../lib/functions/server')
 const Command = require('../../utils/command')
@@ -35,47 +34,61 @@ const startStaticServer = async ({ settings, log }) => {
   log(`\n${NETLIFYDEVLOG} Server listening to`, settings.frameworkPort)
 }
 
+const isNonExistingCommandError = ({ command, error }) => {
+  // `ENOENT` is only returned for non Windows systems
+  // See https://github.com/sindresorhus/execa/pull/447
+  if (error.code === 'ENOENT') {
+    return true
+  }
+
+  // if the command is a package manager we let it report the error
+  if (['yarn', 'npm'].includes(command)) {
+    return false
+  }
+
+  // this only works on English versions of Windows
+  return (
+    typeof error.message === 'string' && error.message.includes('is not recognized as an internal or external command')
+  )
+}
+
 const startFrameworkServer = async function ({ settings, log, exit }) {
   if (settings.noCmd) {
     return await startStaticServer({ settings, log })
   }
 
   log(`${NETLIFYDEVLOG} Starting Netlify Dev with ${settings.framework || 'custom config'}`)
-  const commandBin = await which(settings.command).catch((error) => {
-    if (error.code === 'ENOENT') {
-      throw new Error(
-        `"${settings.command}" could not be found in your PATH. Please make sure that "${settings.command}" is installed and available in your PATH`,
-      )
-    }
-    throw error
-  })
-  const ps = childProcess.spawn(commandBin, settings.args, {
-    env: { ...process.env, ...settings.env, FORCE_COLOR: 'true' },
-    stdio: 'pipe',
-  })
 
-  ps.stdout.pipe(stripAnsiCc.stream()).pipe(process.stdout)
-  ps.stderr.pipe(stripAnsiCc.stream()).pipe(process.stderr)
-
-  process.stdin.pipe(process.stdin)
+  // we use reject=false to avoid rejecting synchronously when the command doesn't exist
+  const frameworkProcess = execa(settings.command, settings.args, { preferLocal: true, reject: false })
+  frameworkProcess.stdout.pipe(stripAnsiCc.stream()).pipe(process.stdout)
+  frameworkProcess.stderr.pipe(stripAnsiCc.stream()).pipe(process.stderr)
+  process.stdin.pipe(frameworkProcess.stdin)
+
+  // we can't try->await->catch since we don't want to block on the framework server which
+  // is a long running process
+  // eslint-disable-next-line promise/catch-or-return,promise/prefer-await-to-then
+  frameworkProcess.then(async () => {
+    const result = await frameworkProcess
+    // eslint-disable-next-line promise/always-return
+    if (result.failed && isNonExistingCommandError({ command: settings.command, error: result })) {
+      log(
+        NETLIFYDEVERR,
+        `Failed launching framework server. Please verify ${chalk.magenta(`'${settings.command}'`)} exists`,
+      )
+    } else {
+      const commandWithArgs = `${settings.command} ${settings.args.join(' ')}`
+      const errorMessage = result.failed
+        ? `${NETLIFYDEVERR} ${result.shortMessage}`
+        : `${NETLIFYDEVWARN} "${commandWithArgs}" exited with code ${result.exitCode}`
 
-  const handleProcessExit = function (code) {
-    log(
-      code > 0 ? NETLIFYDEVERR : NETLIFYDEVWARN,
-      `"${[settings.command, ...settings.args].join(' ')}" exited with code ${code}. Shutting down Netlify Dev server`,
-    )
+      log(`${errorMessage}. Shutting down Netlify Dev server`)
+    }
     process.exit(1)
-  }
-  ps.on('close', handleProcessExit)
-  ps.on('SIGINT', handleProcessExit)
-  ps.on('SIGTERM', handleProcessExit)
+  })
   ;['SIGINT', 'SIGTERM', 'SIGQUIT', 'SIGHUP', 'exit'].forEach((signal) => {
     process.on(signal, () => {
-      try {
-        process.kill(-ps.pid)
-      } catch (error) {
-        // Ignore
-      }
+      frameworkProcess.kill('SIGTERM', { forceKillAfterTimeout: 500 })
       process.exit()
     })
   })
@@ -96,8 +109,6 @@ const startFrameworkServer = async function ({ settings, log, exit }) {
     log(NETLIFYDEVERR, `Please make sure your framework server is running on port ${settings.frameworkPort}`)
     exit(1)
   }
-
-  return ps
 }
 
 // 10 minutes

src/commands/dev/index.js

@@ -177,7 +177,32 @@ test('should start custom command if framework=#custom, command and targetPort a
 
     const error = await t.throwsAsync(() =>
       withDevServer(
-        { cwd: builder.directory, args: ['--command', 'cat non-existing', '--targetPort', '3000'] },
+        { cwd: builder.directory, args: ['--command', 'echo hello', '--targetPort', '3000'] },
+        () => {},
+        true,
+      ),
+    )
+    t.snapshot(normalize(error.stdout))
+  })
+})
+
+test(`should print specific error when command doesn't exist`, async (t) => {
+  await withSiteBuilder('site-with-custom-framework', async (builder) => {
+    await builder.buildAsync()
+
+    const error = await t.throwsAsync(() =>
+      withDevServer(
+        {
+          cwd: builder.directory,
+          args: [
+            '--command',
+            'oops-i-did-it-again forgot-to-use-a-valid-command',
+            '--targetPort',
+            '3000',
+            '--framework',
+            '#custom',
+          ],
+        },
         () => {},
         true,
       ),

tests/framework-detection.test.js

@@ -115,7 +115,7 @@ Generated by [AVA](https://avajs.dev).
     > start␊
     > react-scripts start␊
     ␊
-    ◈ "npm start" exited with code *. Shutting down Netlify Dev server`
+    ◈ Command failed with exit code *: npm start. Shutting down Netlify Dev server`
 
 ## should throw if framework=#custom but command is missing
 
@@ -136,9 +136,19 @@ Generated by [AVA](https://avajs.dev).
 > Snapshot 1
 
     `◈ Netlify Dev ◈␊
-    ◈ Overriding command with setting derived from netlify.toml [dev] block: cat non-existing␊
+    ◈ Overriding command with setting derived from netlify.toml [dev] block: echo hello␊
     ◈ Starting Netlify Dev with #custom␊
-    ◈ "cat non-existing" exited with code *. Shutting down Netlify Dev server`
+    hello␊
+    ◈ "echo hello" exited with code *. Shutting down Netlify Dev server`
+
+## should print specific error when command doesn't exist
+
+> Snapshot 1
+
+    `◈ Netlify Dev ◈␊
+    ◈ Overriding command with setting derived from netlify.toml [dev] block: oops-i-did-it-again forgot-to-use-a-valid-command␊
+    ◈ Starting Netlify Dev with #custom␊
+    ◈ Failed launching framework server. Please verify 'oops-i-did-it-again' exists`
 
 ## should prompt when multiple frameworks are detected
 
@@ -158,4 +168,4 @@ Generated by [AVA](https://avajs.dev).
     > start␊
     > react-scripts start␊
     ␊
-    ◈ "npm start" exited with code *. Shutting down Netlify Dev server`
+    ◈ Command failed with exit code *: npm start. Shutting down Netlify Dev server`

tests/snapshots/framework-detection.test.js.md

@@ -7,7 +7,7 @@ const normalizers = [
   { pattern: /\r\n/gu, value: '\n' },
   { pattern: /❯/gu, value: '>' },
   // normalize exit code from different OSes
-  { pattern: /exited with code \d+/, value: 'exited with code *' },
+  { pattern: /code \d+/, value: 'code *' },
   // this is specific to npm v6
   { pattern: /@ start.+\/.+netlify-cli-tests-v10.+/, value: 'start' },
 ]