From ff4fff6b055c8996e03c1577071fb9d59b9aa109 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paulo=20Ara=C3=BAjo?= Date: Mon, 3 Aug 2026 11:52:44 +0200 Subject: [PATCH] fix: prevent file descriptor exhaustion when watching for EF changes --- src/lib/edge-functions/registry.ts | 77 +++++++-- src/utils/multimap.ts | 4 + .../lib/edge-functions/watch-ignore.test.ts | 13 +- .../unit/lib/edge-functions/watchers.test.ts | 159 ++++++++++++++++++ 4 files changed, 240 insertions(+), 13 deletions(-) create mode 100644 tests/unit/lib/edge-functions/watchers.test.ts diff --git a/src/lib/edge-functions/registry.ts b/src/lib/edge-functions/registry.ts index f08e0ee76f4..1c06e31cd0f 100644 --- a/src/lib/edge-functions/registry.ts +++ b/src/lib/edge-functions/registry.ts @@ -1,6 +1,6 @@ import { readFile } from 'fs/promises' import { statSync } from 'fs' -import { join, resolve } from 'path' +import { join, resolve, sep } from 'path' import { fileURLToPath } from 'url' import type { Declaration, EdgeFunction, FunctionConfig, Manifest, ModuleGraph } from '@netlify/edge-bundler' @@ -128,7 +128,12 @@ export class EdgeFunctionsRegistryImpl implements EdgeFunctionsRegistry { // Mapping file URLs to names of functions that use them as dependencies. private dependencyPaths = new MultiMap() - private directoryWatchers = new Map() + private functionsWatcher?: import('chokidar').FSWatcher + + // Dependency files outside the edge function directories that are being + // explicitly watched, so we can unwatch them when they stop being imported. + private watchedDependencyPaths = new Set() + private env: Record private featureFlags: FeatureFlags @@ -609,6 +614,8 @@ export class EdgeFunctionsRegistryImpl implements EdgeFunctionsRegistry { this.dependencyPaths.add(dependencyPath, functionName) }) }) + + this.syncDependencyWatchers() } /** @@ -704,11 +711,13 @@ export class EdgeFunctionsRegistryImpl implements EdgeFunctionsRegistry { } private async setupWatchers() { - // While functions are guaranteed to be inside one of the configured - // directories, they might be importing files that are located in - // parent directories. So we watch the entire project directory for - // changes. - await this.setupWatcherForDirectory() + // Watching the entire project directory would open one file descriptor + // per file on some platforms, which exhausts the file descriptor table + // in large projects and makes any subsequent `spawn` fail with EBADF. + // Instead, we watch the edge function directories and explicitly watch + // any files outside of them that functions import (see + // `syncDependencyWatchers`). + await this.setupFunctionsWatcher() if (!this.configPath) { return @@ -727,7 +736,23 @@ export class EdgeFunctionsRegistryImpl implements EdgeFunctionsRegistry { }) } - private async setupWatcherForDirectory() { + private get edgeFunctionsDirectories() { + const directories = [getInternalEdgeFunctionsDirectory(this.command)] + + if (this.usesFrameworksAPI) { + directories.push(getFrameworkEdgeFunctionsDirectory(this.command)) + } + + const userFunctionsDirectory = getUserEdgeFunctionsDirectory(this.command) + + if (userFunctionsDirectory !== undefined) { + directories.push(userFunctionsDirectory) + } + + return directories + } + + private async setupFunctionsWatcher() { const toIgnoredRegex = (dir: string) => new RegExp(`^${dir.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}(/|$)`) const toIgnoredEntry = (p: string): string | RegExp => { @@ -745,14 +770,46 @@ export class EdgeFunctionsRegistryImpl implements EdgeFunctionsRegistry { ...this.watchIgnore.map(toIgnoredEntry), this.internalImportMapPath, ] - const watcher = await watchDebounced(this.projectDir, { + + this.functionsWatcher = await watchDebounced(this.edgeFunctionsDirectories, { ignored, onAdd: () => this.checkForAddedOrDeletedFunctions(), onChange: (paths) => this.handleFileChange(paths), onUnlink: () => this.checkForAddedOrDeletedFunctions(), }) - this.directoryWatchers.set(this.projectDir, watcher) + // The initial build may have finished before the watcher was created, in + // which case its dependencies haven't been picked up by a sync yet. + this.syncDependencyWatchers() + } + + private syncDependencyWatchers() { + const watcher = this.functionsWatcher + + if (watcher === undefined) { + return + } + + const directories = this.edgeFunctionsDirectories + const dependencyPaths = new Set( + [...this.dependencyPaths.keys()].filter( + (path) => !directories.some((directory) => path.startsWith(`${directory}${sep}`)), + ), + ) + + this.watchedDependencyPaths.forEach((path) => { + if (!dependencyPaths.has(path)) { + watcher.unwatch(path) + } + }) + + dependencyPaths.forEach((path) => { + if (!this.watchedDependencyPaths.has(path)) { + watcher.add(path) + } + }) + + this.watchedDependencyPaths = dependencyPaths } // We only take into account edge functions from the Frameworks API in diff --git a/src/utils/multimap.ts b/src/utils/multimap.ts index 9ca5e9430a7..1266a37f0f2 100644 --- a/src/utils/multimap.ts +++ b/src/utils/multimap.ts @@ -8,4 +8,8 @@ export class MultiMap { get(key: K): readonly V[] { return this.map.get(key) ?? [] } + + keys(): IterableIterator { + return this.map.keys() + } } diff --git a/tests/unit/lib/edge-functions/watch-ignore.test.ts b/tests/unit/lib/edge-functions/watch-ignore.test.ts index 797f765e19e..fb53e91cabf 100644 --- a/tests/unit/lib/edge-functions/watch-ignore.test.ts +++ b/tests/unit/lib/edge-functions/watch-ignore.test.ts @@ -6,6 +6,7 @@ import { beforeEach, describe, expect, test, vi } from 'vitest' import type BaseCommand from '../../../../src/commands/base-command.js' import { EdgeFunctionsRegistryImpl } from '../../../../src/lib/edge-functions/registry.js' import type { NormalizedCachedConfigConfig } from '../../../../src/utils/command-helpers.js' +import { MultiMap } from '../../../../src/utils/multimap.js' vi.mock('fs', async (importOriginal) => { const actual = await importOriginal() @@ -21,12 +22,18 @@ vi.mock('@netlify/dev-utils', async (importOriginal) => { }) // Creates a partial registry via Object.create so the constructor is bypassed, -// then populates the private fields needed by setupWatcherForDirectory. +// then populates the private fields needed by setupFunctionsWatcher. const makeRegistry = (fields: { projectDir: string; servePath: string; publishDir: string; watchIgnore: string[] }) => { const registry = Object.create(EdgeFunctionsRegistryImpl.prototype) as EdgeFunctionsRegistryImpl Object.assign(registry, { ...fields, - directoryWatchers: new Map(), + command: { + name: () => 'dev', + workingDir: fields.projectDir, + netlify: { config: { build: { edge_functions: join(fields.projectDir, 'netlify/edge-functions') } } }, + }, + dependencyPaths: new MultiMap(), + watchedDependencyPaths: new Set(), checkForAddedOrDeletedFunctions: vi.fn(), handleFileChange: vi.fn(), }) @@ -36,7 +43,7 @@ const makeRegistry = (fields: { projectDir: string; servePath: string; publishDi const captureIgnored = async (registry: EdgeFunctionsRegistryImpl): Promise<(string | RegExp)[]> => { const { watchDebounced } = await import('@netlify/dev-utils') vi.mocked(watchDebounced).mockClear() - await (registry as unknown as { setupWatcherForDirectory: () => Promise }).setupWatcherForDirectory() + await (registry as unknown as { setupFunctionsWatcher: () => Promise }).setupFunctionsWatcher() const [, options] = vi.mocked(watchDebounced).mock.calls[0] return (options as { ignored: (string | RegExp)[] }).ignored } diff --git a/tests/unit/lib/edge-functions/watchers.test.ts b/tests/unit/lib/edge-functions/watchers.test.ts new file mode 100644 index 00000000000..9060cf0a714 --- /dev/null +++ b/tests/unit/lib/edge-functions/watchers.test.ts @@ -0,0 +1,159 @@ +import { join, resolve } from 'path' +import { pathToFileURL } from 'url' + +import { describe, expect, test, vi } from 'vitest' + +import type BaseCommand from '../../../../src/commands/base-command.js' +import { EdgeFunctionsRegistryImpl } from '../../../../src/lib/edge-functions/registry.js' +import { MultiMap } from '../../../../src/utils/multimap.js' + +vi.mock('@netlify/dev-utils', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + watchDebounced: vi.fn().mockResolvedValue({ close: vi.fn(), add: vi.fn(), unwatch: vi.fn() }), + } +}) + +const projectDir = resolve('/project') +const userFunctionsDir = join(projectDir, 'netlify', 'edge-functions') +const internalFunctionsDir = join(projectDir, '.netlify', 'edge-functions') +const frameworkFunctionsDir = join(projectDir, '.netlify', 'v1', 'edge-functions') +const functionPath = join(userFunctionsDir, 'func1.ts') +const insideDependencyPath = join(userFunctionsDir, 'helper.ts') +const outsideDependencyPath = join(projectDir, 'shared', 'util.ts') +const staleDependencyPath = join(projectDir, 'old-dep.ts') + +const makeCommand = (name = 'dev') => + ({ + name: () => name, + workingDir: projectDir, + netlify: { + config: { build: { edge_functions: userFunctionsDir } }, + frameworksAPIPaths: { edgeFunctions: { path: frameworkFunctionsDir } }, + }, + } as unknown as BaseCommand) + +const makeRegistry = (overrides: Record = {}) => { + const registry = Object.create(EdgeFunctionsRegistryImpl.prototype) as EdgeFunctionsRegistryImpl + Object.assign(registry, { + command: makeCommand(), + projectDir, + servePath: join(projectDir, '.netlify', 'edge-functions-serve'), + publishDir: join(projectDir, '_site'), + watchIgnore: [], + configPath: '', + internalFunctions: [], + userFunctions: [], + functionPaths: new Map(), + dependencyPaths: new MultiMap(), + watchedDependencyPaths: new Set(), + checkForAddedOrDeletedFunctions: vi.fn(), + handleFileChange: vi.fn(), + ...overrides, + }) + return registry +} + +type RegistryInternals = { + setupWatchers: () => Promise + processGraph: (graph: unknown) => void + watchedDependencyPaths: Set +} + +const asInternals = (registry: EdgeFunctionsRegistryImpl) => registry as unknown as RegistryInternals + +describe('setupWatchers', () => { + test('watches the edge function directories and not the project directory', async () => { + const { watchDebounced } = await import('@netlify/dev-utils') + vi.mocked(watchDebounced).mockClear() + + const registry = makeRegistry() + await asInternals(registry).setupWatchers() + + const watchedTargets = vi.mocked(watchDebounced).mock.calls.map(([target]) => target) + expect(watchedTargets).not.toContainEqual(projectDir) + + const directoriesTarget = watchedTargets.find((target) => Array.isArray(target)) + expect(directoriesTarget).toEqual(expect.arrayContaining([internalFunctionsDir, userFunctionsDir])) + expect(directoriesTarget).not.toContain(projectDir) + }) + + test('includes the frameworks API directory when running serve', async () => { + const { watchDebounced } = await import('@netlify/dev-utils') + vi.mocked(watchDebounced).mockClear() + + const registry = makeRegistry({ command: makeCommand('serve') }) + await asInternals(registry).setupWatchers() + + const directoriesTarget = vi + .mocked(watchDebounced) + .mock.calls.map(([target]) => target) + .find(Array.isArray) + expect(directoriesTarget).toContain(frameworkFunctionsDir) + }) +}) + +describe('dependency watching', () => { + const makeGraph = (dependencyPaths: string[]) => ({ + modules: [ + { + specifier: pathToFileURL(functionPath).href, + dependencies: dependencyPaths.map((path) => ({ code: { specifier: pathToFileURL(path).href } })), + }, + ...dependencyPaths.map((path) => ({ specifier: pathToFileURL(path).href, dependencies: [] })), + ], + }) + + const makeRegistryWithWatcher = () => { + const functionsWatcher = { + add: vi.fn<(path: string) => void>(), + unwatch: vi.fn<(path: string) => void>(), + close: vi.fn(), + } + const registry = makeRegistry({ + functionsWatcher, + functionPaths: new Map([[functionPath, 'func1']]), + }) + return { registry, functionsWatcher } + } + + test('watches dependencies that live outside the edge function directories', () => { + const { registry, functionsWatcher } = makeRegistryWithWatcher() + + asInternals(registry).processGraph(makeGraph([outsideDependencyPath])) + + const addedPaths = functionsWatcher.add.mock.calls.flatMap(([path]) => path) + expect(addedPaths).toContain(outsideDependencyPath) + }) + + test('does not explicitly watch dependencies inside the edge function directories', () => { + const { registry, functionsWatcher } = makeRegistryWithWatcher() + + asInternals(registry).processGraph(makeGraph([insideDependencyPath])) + + const addedPaths = functionsWatcher.add.mock.calls.flatMap(([path]) => path) + expect(addedPaths).not.toContain(insideDependencyPath) + }) + + test('unwatches dependencies that are no longer part of the graph', () => { + const { registry, functionsWatcher } = makeRegistryWithWatcher() + asInternals(registry).watchedDependencyPaths = new Set([staleDependencyPath]) + + asInternals(registry).processGraph(makeGraph([outsideDependencyPath])) + + const unwatchedPaths = functionsWatcher.unwatch.mock.calls.flatMap(([path]) => path) + expect(unwatchedPaths).toContain(staleDependencyPath) + }) + + test('keeps watching dependencies that remain in the graph', () => { + const { registry, functionsWatcher } = makeRegistryWithWatcher() + asInternals(registry).watchedDependencyPaths = new Set([outsideDependencyPath]) + + asInternals(registry).processGraph(makeGraph([outsideDependencyPath])) + + expect(functionsWatcher.unwatch).not.toHaveBeenCalled() + const addedPaths = functionsWatcher.add.mock.calls.flatMap(([path]) => path) + expect(addedPaths).not.toContain(outsideDependencyPath) + }) +})