fix: leaky file handles for aborted requests (#21)

lib/fs-proxy.js

@@ -1,3 +1,4 @@
+import { createReadStream } from 'node:fs';
 import { access, constants, lstat, open, readdir, readFile, realpath } from 'node:fs/promises';
 import { createRequire } from 'node:module';
 import { join, sep as dirSep } from 'node:path';
@@ -53,6 +54,9 @@ export const fsProxy = {
 	async readFile(filePath) {
 		return readFile(filePath);
 	},
+	readStream(filePath) {
+		return createReadStream(filePath, { autoClose: true });
+	},
 	async realpath(filePath) {
 		try {
 			const real = await realpath(filePath);

lib/resolver.js

@@ -58,6 +58,13 @@ export class FileResolver {
 		return this.#fs.readFile(filePath);
 	}
 
+	/**
+	 * @param {string} filePath
+	 */
+	readStream(filePath) {
+		return this.#fs.readStream(filePath);
+	}
+
 	/**
 	 * @param {string} url
 	 * @returns {Promise<ResolveResult>}

lib/server.js

@@ -19,24 +19,21 @@ import { headerCase, strBytes } from './utils.js';
 @typedef {import('./types.js').ResolvedFile} ResolvedFile
 @typedef {import('./types.js').ResolveResult} ResolveResult
 @typedef {import('./types.js').ServerOptions} ServerOptions
-@typedef {'error' | 'headers' | 'list' | 'file'} ResponseMode
 **/
 
 /**
  * @param {ServerOptions} options
  * @param {{ logNetwork?: (data: ReqResMeta) => void }} [callbacks]
  * @returns {Server}
  */
-export function staticServer(options, callbacks) {
+export function staticServer(options, { logNetwork } = {}) {
 	const resolver = new FileResolver(options, fsProxy);
-	const handlerOptions = { ...options, streaming: true };
 
 	return createServer(async (req, res) => {
-		const handler = new RequestHandler({ req, res }, resolver, handlerOptions);
-		res.on('close', () => {
-			handler.endedAt = Date.now();
-			callbacks?.logNetwork?.(handler.data);
-		});
+		const handler = new RequestHandler({ req, res }, resolver, options);
+		if (typeof logNetwork === 'function') {
+			res.on('close', () => logNetwork(handler.data()));
+		}
 		await handler.process();
 	});
 }
@@ -48,7 +45,7 @@ export class RequestHandler {
 	#resolver;
 
 	/** @type {number} */
-	startedAt = Date.now();
+	startedAt;
 	/** @type {number | undefined} */
 	endedAt;
 	/** @type {string} */
@@ -60,8 +57,6 @@ export class RequestHandler {
 	 * @type {ResolvedFile | null}
 	 */
 	file = null;
-	/** @type {ResponseMode} */
-	mode = 'error';
 	/**
 	 * Error that may be logged to the terminal
 	 * @type {Error | string | undefined}
@@ -71,14 +66,19 @@ export class RequestHandler {
 	/**
 	 * @param {{ req: IncomingMessage, res: ServerResponse }} reqRes
 	 * @param {FileResolver} resolver
-	 * @param {ServerOptions & {streaming: boolean}} options
+	 * @param {ServerOptions & {_dryRun?: boolean}} options
 	 */
 	constructor({ req, res }, resolver, options) {
 		this.#req = req;
 		this.#res = res;
 		this.#resolver = resolver;
 		this.#options = options;
-		this.status = 404;
+
+		this.startedAt = Date.now();
+		res.on('close', async () => {
+			this.endedAt = Date.now();
+		});
+
 		if (req.url) {
 			this.url = req.url;
 			this.urlPath = req.url.split(/[\?\#]/)[0];
@@ -92,6 +92,7 @@ export class RequestHandler {
 		return this.#res.statusCode;
 	}
 	set status(code) {
+		if (this.#res.headersSent) return;
 		this.#res.statusCode = code;
 	}
 	get headers() {
@@ -101,8 +102,8 @@ export class RequestHandler {
 	async process() {
 		// bail for unsupported http methods
 		if (!SUPPORTED_METHODS.includes(this.method)) {
-			this.error = new Error(`HTTP method ${this.method} is not supported`);
 			this.status = 405;
+			this.error = new Error(`HTTP method ${this.method} is not supported`);
 			return this.#sendErrorPage();
 		}
 
@@ -135,53 +136,50 @@ export class RequestHandler {
 	 * @param {ResolvedFile} file
 	 */
 	async #sendFile(file) {
-		const { method } = this;
 		/** @type {FileHandle | undefined} */
 		let handle;
 		/** @type {string | undefined} */
 		let contentType;
 		/** @type {number | undefined} */
 		let contentLength;
-
 		try {
 			// check that we can actually open the file
 			// (especially on windows where it might be busy)
 			handle = await this.#resolver.open(file.filePath);
 			contentType = await getContentType({ filePath: file.filePath, fileHandle: handle });
 			contentLength = (await handle.stat()).size;
 		} catch (/** @type {any} */ err) {
-			this.status = 500;
-			if (err?.syscall === 'open') {
-				if (err.code === 'EBUSY') this.status = 403;
-				handle?.close();
+			if (err?.syscall === 'open' && err.code === 'EBUSY') {
+				this.status = err?.syscall === 'open' && err.code === 'EBUSY' ? 403 : 500;
 			}
 			if (err?.message) {
 				this.error = err;
 			}
+		} finally {
+			await handle?.close();
+		}
+
+		if (this.status >= 400) {
 			return this.#sendErrorPage();
 		}
 
+		if (this.method === 'OPTIONS') {
+			this.status = 204;
+		}
+
 		this.#setHeaders(file.localPath ?? file.filePath, {
 			contentType,
 			contentLength,
 			cors: this.#options.cors,
 			headers: this.#options.headers,
 		});
 
-		if (method === 'OPTIONS') {
-			handle.close();
-			this.status = 204;
-			this.#send();
-		} else if (method === 'HEAD') {
-			handle.close();
-			this.#send();
-		} else if (this.#options.streaming === false) {
-			handle.close();
-			const buffer = await this.#resolver.read(file.filePath);
-			this.#send(buffer);
+		if (this.#options._dryRun) {
+			return;
+		} else if (this.method === 'OPTIONS' || this.method === 'HEAD') {
+			return this.#send();
 		} else {
-			const stream = handle.createReadStream({ autoClose: true, start: 0 });
-			this.#send(stream);
+			return this.#send(this.#resolver.readStream(file.filePath));
 		}
 	}
 
@@ -216,7 +214,7 @@ export class RequestHandler {
 			cors: this.#options.cors,
 			headers: [],
 		});
-		this.#send(body);
+		return this.#send(body);
 	}
 
 	/**
@@ -225,44 +223,53 @@ export class RequestHandler {
 	#send(contents) {
 		if (this.method === 'HEAD' || this.method === 'OPTIONS') {
 			this.#res.end();
-		} else {
-			if (typeof contents === 'string' || Buffer.isBuffer(contents)) {
-				this.#res.write(contents);
-				this.#res.end();
-			} else if (typeof contents?.pipe === 'function') {
-				contents.pipe(this.#res);
-			}
+		} else if (this.#req.destroyed) {
+			this.#setHeader('content-length', '0');
+			this.#res.end();
+		} else if (typeof contents === 'string' || Buffer.isBuffer(contents)) {
+			this.#res.write(contents);
+			this.#res.end();
+		} else if (typeof contents?.pipe === 'function') {
+			contents.pipe(this.#res);
 		}
 	}
 
+	/**
+	 * @param {string} name
+	 * @param {number | string | string[]} value
+	 */
+	#setHeader(name, value) {
+		if (this.#res.headersSent) return;
+		this.#res.setHeader(headerCase(name), value);
+	}
+
 	/**
 	 * @param {string} localPath
 	 * @param {Partial<{ contentType: string, contentLength: number; cors: boolean; headers: ServerOptions['headers'] }>} options
 	 */
 	#setHeaders(localPath, { contentLength, contentType, cors, headers }) {
-		const { method, status } = this;
-		const isOptions = method === 'OPTIONS';
+		if (this.#res.headersSent) return;
 
-		if (isOptions || status === 405) {
+		const isOptions = this.method === 'OPTIONS';
+		const headerRules = headers ?? this.#options.headers;
+
+		if (isOptions || this.status === 405) {
 			this.#setHeader('allow', SUPPORTED_METHODS.join(', '));
 		}
 		if (!isOptions) {
 			contentType ??= typeForFilePath(localPath).toString();
 			this.#setHeader('content-type', contentType);
 		}
-		if (isOptions || status === 204) {
+		if (isOptions || this.status === 204) {
 			contentLength = 0;
 		}
 		if (typeof contentLength === 'number') {
 			this.#setHeader('content-length', String(contentLength));
 		}
-
 		if (cors ?? this.#options.cors) {
 			this.#setCorsHeaders();
 		}
-
-		const headerRules = headers ?? this.#options.headers;
-		if (headerRules.length) {
+		if (localPath && headerRules.length) {
 			for (const { name, value } of fileHeaders(localPath, headerRules)) {
 				this.#res.setHeader(name, value);
 			}
@@ -285,16 +292,8 @@ export class RequestHandler {
 		}
 	}
 
-	/**
-	 * @param {string} name
-	 * @param {string} value
-	 */
-	#setHeader(name, value) {
-		this.#res.setHeader(headerCase(name), value);
-	}
-
 	/** @returns {ReqResMeta} */
-	get data() {
+	data() {
 		const { startedAt, endedAt, status, method, url, urlPath, file, error } = this;
 		return { startedAt, endedAt, status, method, url, urlPath, file, error };
 	}

lib/types.js

@@ -23,6 +23,7 @@
 	open(filePath: string): Promise<import('node:fs/promises').FileHandle>;
 	readable(filePath: string, kind?: FSEntryKind | null): Promise<boolean>;
 	readFile(filePath: string): Promise<import('node:buffer').Buffer | string>;
+	readStream(filePath: string): import('node:fs').ReadStream;
 	realpath(filePath: string): Promise<string | null>;
 }} FSProxy
 

test/server.test.js

@@ -60,7 +60,7 @@ function mockReqRes(method, url, headers = {}) {
  */
 function withHandlerContext(options, files) {
 	const resolver = getResolver(options, files);
-	const handlerOptions = { ...options, streaming: false };
+	const handlerOptions = { ...options, _dryRun: true };
 
 	return (method, url, headers) => {
 		const { req, res } = mockReqRes(method, url, headers);
@@ -125,12 +125,12 @@ suite('staticServer', () => {
 });
 
 suite('RequestHandler.constructor', () => {
-	test('starts with a 404 status', async () => {
-		const options = { ...blankOptions, streaming: false };
+	test('starts with a 200 status', async () => {
+		const options = { ...blankOptions, _dryRun: true };
 		const handler = new RequestHandler(mockReqRes('GET', '/'), getResolver(), options);
 		strictEqual(handler.method, 'GET');
 		strictEqual(handler.urlPath, '/');
-		strictEqual(handler.status, 404);
+		strictEqual(handler.status, 200);
 		strictEqual(handler.file, null);
 	});
 });
@@ -157,7 +157,7 @@ suite('RequestHandler.process', async () => {
 		test(`${method} method is unsupported`, async () => {
 			const handler = request(method, '/README.md');
 			strictEqual(handler.method, method);
-			strictEqual(handler.status, 404);
+			strictEqual(handler.status, 200);
 			strictEqual(handler.urlPath, '/README.md');
 			strictEqual(handler.file, null);
 
@@ -171,14 +171,8 @@ suite('RequestHandler.process', async () => {
 
 	test('GET resolves a request with an index file', async () => {
 		const handler = request('GET', '/');
-
-		// Initial status is 404
-		strictEqual(handler.method, 'GET');
-		strictEqual(handler.status, 404);
-		strictEqual(typeof handler.startedAt, 'number');
-
-		// Processing the request finds the index.html file
 		await handler.process();
+
 		strictEqual(handler.status, 200);
 		strictEqual(handler.file?.kind, 'file');
 		strictEqual(handler.file?.localPath, 'index.html');

test/shared.js

@@ -154,6 +154,10 @@ export function testFsProxy(filePaths = {}) {
 		async readFile(filePath) {
 			return readFile(filePath);
 		},
+		// @ts-expect-error (memfs ReadStream doesn't have ReadStream#close)
+		readStream(filePath) {
+			return fs.createReadStream(filePath, { autoClose: true, start: 0 });
+		},
 		async readable(filePath, kind) {
 			if (kind === undefined) {
 				kind = await this.kind(filePath);