Repository: vercel/next.js
Pull request: Exit `next dev` with a non-zero code when next.config.js is invalid

In `next dev`, we're currently ignoring the exit code of the spawned child process, and always exiting with `0` instead. We should exit the process with the child's exit code instead. This is relevant when the `next.config.js` is invalid, for example.

Changed files:
filename: packages/next/src/cli/next-dev.ts
patch: @@ -76,6 +76,10 @@ const handleSessionStop = async (signal: NodeJS.Signals | number | null) => {
   if (sessionStopHandled) return
   sessionStopHandled = true
 
+  // Capture the child's exit code if it has already exited and caused the
+  // session stop (via the 'exit' event), otherwise assume success (0).
+  const exitCode = child?.exitCode || 0
+
   if (
     signal != null &&
     child?.pid &&
@@ -150,7 +154,7 @@ const handleSessionStop = async (signal: NodeJS.Signals | number | null) => {
   // the program, or the cursor could remain hidden
   process.stdout.write('\x1B[?25h')
   process.stdout.write('\n')
-  process.exit(0)
+  process.exit(exitCode)
 }
 
 process.on('SIGINT', () => handleSessionStop('SIGINT'))

filename: packages/next/src/server/lib/start-server.ts
patch: @@ -367,22 +367,22 @@ export async function startServer(
       // Only load env and config in dev to for logging purposes
       let envInfo: string[] | undefined
       let experimentalFeatures: ConfiguredExperimentalFeature[] | undefined
-      if (isDev) {
-        const startServerInfo = await getStartServerInfo({ dir, dev: isDev })
-        envInfo = startServerInfo.envInfo
-        experimentalFeatures = startServerInfo.experimentalFeatures
-      }
-      logStartInfo({
-        networkUrl,
-        appUrl,
-        envInfo,
-        experimentalFeatures,
-        logBundler: isDev,
-      })
+      try {
+        if (isDev) {
+          const startServerInfo = await getStartServerInfo({ dir, dev: isDev })
+          envInfo = startServerInfo.envInfo
+          experimentalFeatures = startServerInfo.experimentalFeatures
+        }
+        logStartInfo({
+          networkUrl,
+          appUrl,
+          envInfo,
+          experimentalFeatures,
+          logBundler: isDev,
+        })
 
-      Log.event(`Starting...`)
+        Log.event(`Starting...`)
 
-      try {
         let cleanupStarted = false
         let closeUpgraded: (() => void) | null = null
         const cleanup = () => {

filename: test/e2e/build-indicator/test/index.test.ts
patch: @@ -1,7 +1,7 @@
 /* eslint-env jest */
 import { join } from 'path'
 import { retry } from 'next-test-utils'
-import { nextTestSetup, isNextDev, isNextStart } from 'e2e-utils'
+import { nextTestSetup, isNextDev, isNextDeploy } from 'e2e-utils'
 
 const installCheckVisible = (browser) => {
   return browser.eval(`(function() {
@@ -19,24 +19,34 @@ const installCheckVisible = (browser) => {
 
 describe('Build Activity Indicator', () => {
   // Use describe.skip so that this suite does not fail with "no tests" during deploy tests.
-  ;(isNextStart ? describe : describe.skip)('Invalid position config', () => {
+  ;(isNextDeploy ? describe.skip : describe)('Invalid position config', () => {
     const { next } = nextTestSetup({
       files: join(__dirname, '..'),
       skipStart: true,
-      startServerTimeout: 1000,
       nextConfig: {
         devIndicators: {
           // Intentionally invalid position to test error
-          position: 'ttop-leff' as any,
+          // @ts-expect-error
+          position: 'ttop-leff',
         },
       },
     })
 
     it('should validate position config', async () => {
-      const result = await next.build()
+      if (isNextDev) {
+        try {
+          await next.start()
+        } catch (err) {
+          expect(err).toEqual(
+            new Error('next dev exited unexpectedly with code/signal 1')
+          )
+        }
+      } else {
+        const result = await next.build()
+        expect(result.exitCode).toBe(1)
+      }
 
-      expect(result.exitCode).toBe(1)
-      expect(result.cliOutput).toContain(
+      expect(next.cliOutput).toContain(
         `Invalid "devIndicator.position" provided, expected one of top-left, top-right, bottom-left, bottom-right, received ttop-leff`
       )
     })

filename: test/lib/next-modes/next-dev.ts
patch: @@ -81,20 +81,24 @@ export class NextDevInstance extends NextInstance {
           this.emit('stderr', [msg])
         })
 
+        const serverReadyTimeoutId = this.setServerReadyTimeout(
+          reject,
+          this.startServerTimeout
+        )
+
         this.childProcess.on('close', (code, signal) => {
           if (this.isStopping) return
           if (code || signal) {
-            require('console').error(
+            this.childProcess = undefined
+            const error = new Error(
               `next dev exited unexpectedly with code/signal ${code || signal}`
             )
+            clearTimeout(serverReadyTimeoutId)
+            require('console').error(error)
+            reject(error)
           }
         })
 
-        const serverReadyTimeoutId = this.setServerReadyTimeout(
-          reject,
-          this.startServerTimeout
-        )
-
         const readyCb = (msg) => {
           const resolveServer = () => {
             clearTimeout(serverReadyTimeoutId)