fix: clean up edge functions server (#310)

* fix: clean up edge functions server

* add escape hatch

* fix: avoid killing the process twice

* swallow error if server has been stopped

Author: eduardoboucas

package-lock.json

@@ -46,6 +46,7 @@
     "@types/node": "^18.19.110",
     "@types/parse-gitignore": "^1.0.2",
     "@types/write-file-atomic": "^4.0.3",
+    "execa": "^8.0.1",
     "tmp-promise": "^3.0.3",
     "tsup": "^8.0.0",
     "vitest": "^3.0.0"
@@ -58,10 +59,11 @@
     "dot-prop": "9.0.0",
     "env-paths": "^3.0.0",
     "find-up": "7.0.0",
-    "js-image-generator": "^1.0.4",
     "image-size": "^2.0.2",
+    "js-image-generator": "^1.0.4",
     "lodash.debounce": "^4.0.8",
     "parse-gitignore": "^2.0.0",
+    "semver": "^7.7.2",
     "uuid": "^11.1.0",
     "write-file-atomic": "^5.0.1"
   }

packages/dev-utils/package.json

@@ -0,0 +1,39 @@
+import { platform } from 'node:os'
+
+import type { ExecaChildProcess } from 'execa'
+import { satisfies } from 'semver'
+
+// 1 second
+const SERVER_KILL_TIMEOUT = 1e3
+
+export interface ProcessRef {
+  ps?: ExecaChildProcess
+}
+
+export const killProcess = (ps?: ExecaChildProcess) => {
+  // If the process is no longer running, there's nothing left to do.
+  if (!ps || ps.exitCode !== null) {
+    return
+  }
+
+  return new Promise<void>((resolve, reject) => {
+    void ps.on('close', () => {
+      resolve()
+    })
+    void ps.on('error', reject)
+
+    // On Windows with Node 21+, there's a bug where attempting to kill a child process
+    // results in an EPERM error. Ignore the error in that case.
+    // See: https://github.com/nodejs/node/issues/51766
+    // We also disable execa's `forceKillAfterTimeout` in this case
+    // which can cause unhandled rejection.
+    try {
+      ps.kill('SIGTERM', {
+        forceKillAfterTimeout:
+          platform() === 'win32' && satisfies(process.version, '>=21') ? false : SERVER_KILL_TIMEOUT,
+      })
+    } catch {
+      // no-op
+    }
+  })
+}

packages/dev-utils/src/lib/process.ts

@@ -10,6 +10,7 @@ export { Handler } from './lib/handler.js'
 export { LocalState } from './lib/local-state.js'
 export { type Logger, netlifyCommand, netlifyCyan, netlifyBanner } from './lib/logger.js'
 export { memoize, MemoizeCache } from './lib/memoize.js'
+export { killProcess, type ProcessRef } from './lib/process.js'
 export { HTTPServer } from './server/http_server.js'
 export { watchDebounced } from './lib/watch-debounced.js'
 

packages/dev-utils/src/main.ts

@@ -470,7 +470,7 @@ export class NetlifyDev {
         return acc
       }, {})
 
-      this.#edgeFunctionsHandler = new EdgeFunctionsHandler({
+      const edgeFunctionsHandler = new EdgeFunctionsHandler({
         configDeclarations: this.#config?.config.edge_functions ?? [],
         directories: [this.#config?.config.build.edge_functions].filter(Boolean) as string[],
         env,
@@ -479,6 +479,9 @@ export class NetlifyDev {
         siteID,
         siteName: config?.siteInfo.name,
       })
+      this.#edgeFunctionsHandler = edgeFunctionsHandler
+
+      this.#cleanupJobs.push(() => edgeFunctionsHandler.stop())
     }
 
     if (this.#features.functions) {

packages/dev/src/main.ts

@@ -1,7 +1,7 @@
 import path from 'node:path'
 import { fileURLToPath, pathToFileURL } from 'node:url'
 
-import { Logger, renderFunctionErrorPage, type Geolocation } from '@netlify/dev-utils'
+import { Logger, renderFunctionErrorPage, killProcess, type Geolocation, type ProcessRef } from '@netlify/dev-utils'
 import {
   find,
   generateManifest,
@@ -46,6 +46,7 @@ export type EdgeFunctionsMatch = Awaited<ReturnType<EdgeFunctionsHandler['getFun
 
 export class EdgeFunctionsHandler {
   private configDeclarations: Declaration[]
+  private denoServerProcess?: ProcessRef
   private directories: string[]
   private geolocation: Geolocation
   private initialization: ReturnType<typeof this.initialize>
@@ -257,7 +258,8 @@ export class EdgeFunctionsHandler {
   private async initialize(env: Record<string, string>) {
     let success = true
 
-    const processRef = {}
+    const processRef: ProcessRef = {}
+    this.denoServerProcess = processRef
 
     // If we ran the server on a random port, we wouldn't know how to reach it.
     // Compute the port upfront and pass it on to the server.
@@ -329,6 +331,11 @@ export class EdgeFunctionsHandler {
         method: 'HEAD',
       })
     } catch {
+      // If we've already stopped the server, swallow the error.
+      if (!this.denoServerProcess) {
+        return
+      }
+
       if ((count + 1) * DENO_SERVER_POLL_INTERVAL > DENO_SERVER_POLL_TIMEOUT) {
         throw new Error('Could not establish a connection to the Netlify Edge Functions local development server')
       }
@@ -338,6 +345,20 @@ export class EdgeFunctionsHandler {
       return this.waitForDenoServer(port, count + 1)
     }
   }
+
+  async stop() {
+    if (!this.denoServerProcess) {
+      return
+    }
+
+    const { ps } = this.denoServerProcess
+
+    this.denoServerProcess = undefined
+
+    try {
+      await killProcess(ps)
+    } catch {}
+  }
 }
 
 export { type Declaration } from '@netlify/edge-bundler'

packages/edge-functions/dev/node/main.ts

@@ -22,12 +22,15 @@ export default function netlify(options: NetlifyPluginOptions = {}): any {
     return []
   }
 
+  let netlifyDev: NetlifyDev | undefined
+
   const plugin: vite.Plugin = {
     name: 'vite-plugin-netlify',
     async configureServer(viteDevServer) {
       const logger = createLoggerFromViteLogger(viteDevServer.config.logger)
       const { blobs, edgeFunctions, functions, middleware = true, redirects, staticFiles } = options
-      const netlifyDev = new NetlifyDev({
+
+      netlifyDev = new NetlifyDev({
         blobs,
         edgeFunctions,
         functions,
@@ -46,6 +49,17 @@ export default function netlify(options: NetlifyPluginOptions = {}): any {
 
       if (middleware) {
         viteDevServer.middlewares.use(async function netlifyPreMiddleware(nodeReq, nodeRes, next) {
+          // This should never happen, but adding this escape hatch just in case.
+          if (!netlifyDev) {
+            logger.error(
+              'Some primitives will not work as expected due to an unknown error. Please restart your application.',
+            )
+
+            next()
+
+            return
+          }
+
           const headers: Record<string, string> = {}
           const result = await netlifyDev.handleAndIntrospectNodeRequest(nodeReq, {
             headersCollector: (key, value) => {
@@ -69,6 +83,7 @@ export default function netlify(options: NetlifyPluginOptions = {}): any {
 
           next()
         })
+
         logger.log(`Middleware loaded. Emulating features: ${netlifyDev.getEnabledFeatures().join(', ')}.`)
       }
 
@@ -78,6 +93,12 @@ export default function netlify(options: NetlifyPluginOptions = {}): any {
         )
       }
     },
+
+    async closeBundle() {
+      await netlifyDev?.stop()
+
+      netlifyDev = undefined
+    },
   }
 
   return [plugin]