quality of life updates for the node adapter (#9582)

* descriptive names for files and functions

* update tests

* add changeset

* appease linter

* Apply suggestions from code review

Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>

* `server-entrypoint.js` -> `server.js`

* prevent crash on stream error (from PR 9533)

* Apply suggestions from code review

Co-authored-by: Luiz Ferraz <luiz@lferraz.com>

* `127.0.0.1` -> `localhost`

* add changeset for fryuni's fix

* Apply suggestions from code review

* Apply suggestions from code review

Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>

---------

Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>
Co-authored-by: Luiz Ferraz <luiz@lferraz.com>
Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>

.changeset/unlucky-stingrays-clean.md

@@ -0,0 +1,5 @@
+---
+"@astrojs/node": patch
+---
+
+Fixes an issue where the preview server appeared to be ready to serve requests before binding to a port.

.changeset/weak-apes-add.md

@@ -0,0 +1,6 @@
+---
+"@astrojs/node": major
+---
+
+**Breaking**: Minimum required Astro version is now 4.2.0.
+Reorganizes internals to be more maintainable.

packages/astro/src/core/app/node.ts

@@ -106,15 +106,20 @@ export class NodeApp extends App {
 			try {
 				const reader = body.getReader();
 				destination.on('close', () => {
-					reader.cancel();
+					// Cancelling the reader may reject not just because of
+					// an error in the ReadableStream's cancel callback, but
+					// also because of an error anywhere in the stream.
+					reader.cancel().catch(err => {
+						console.error(`There was an uncaught error in the middle of the stream while rendering ${destination.req.url}.`, err);
+					});
 				});
 				let result = await reader.read();
 				while (!result.done) {
 					destination.write(result.value);
 					result = await reader.read();
 				}
-			} catch (err: any) {
-				console.error(err?.stack || err?.message || String(err));
+			// the error will be logged by the "on end" callback above
+			} catch {
 				destination.write('Internal server error');
 			}
 		}

packages/integrations/node/src/index.ts

@@ -1,6 +1,7 @@
-import type { AstroAdapter, AstroIntegration } from 'astro';
 import { AstroError } from 'astro/errors';
+import type { AstroAdapter, AstroIntegration } from 'astro';
 import type { Options, UserOptions } from './types.js';
+
 export function getAdapter(options: Options): AstroAdapter {
 	return {
 		name: '@astrojs/node',

packages/integrations/node/src/log-listening-on.ts

@@ -0,0 +1,84 @@
+import os from "node:os";
+import type http from "node:http";
+import https from "node:https";
+import type { AstroIntegrationLogger } from "astro";
+import type { Options } from './types.js';
+import type { AddressInfo } from "node:net";
+
+export async function logListeningOn(logger: AstroIntegrationLogger, server: http.Server | https.Server, options: Pick<Options, "host">) {
+	await new Promise<void>(resolve => server.once('listening', resolve)) 
+    const protocol = server instanceof https.Server ? 'https' : 'http';
+    // Allow to provide host value at runtime
+	const host = getResolvedHostForHttpServer(
+		process.env.HOST !== undefined && process.env.HOST !== '' ? process.env.HOST : options.host
+	);
+    const { port } = server.address() as AddressInfo;
+	const address = getNetworkAddress(protocol, host, port);
+
+	if (host === undefined) {
+		logger.info(
+			`Server listening on \n  local: ${address.local[0]} \t\n  network: ${address.network[0]}\n`
+		);
+	} else {
+		logger.info(`Server listening on ${address.local[0]}`);
+	}
+}
+
+function getResolvedHostForHttpServer(host: string | boolean) {
+	if (host === false) {
+		// Use a secure default
+		return 'localhost';
+	} else if (host === true) {
+		// If passed --host in the CLI without arguments
+		return undefined; // undefined typically means 0.0.0.0 or :: (listen on all IPs)
+	} else {
+		return host;
+	}
+}
+
+interface NetworkAddressOpt {
+	local: string[];
+	network: string[];
+}
+
+const wildcardHosts = new Set(['0.0.0.0', '::', '0000:0000:0000:0000:0000:0000:0000:0000']);
+
+// this code from vite https://github.com/vitejs/vite/blob/d09bbd093a4b893e78f0bbff5b17c7cf7821f403/packages/vite/src/node/utils.ts#L892-L914
+export function getNetworkAddress(
+	protocol: 'http' | 'https' = 'http',
+	hostname: string | undefined,
+	port: number,
+	base?: string
+) {
+	const NetworkAddress: NetworkAddressOpt = {
+		local: [],
+		network: [],
+	};
+	Object.values(os.networkInterfaces())
+		.flatMap((nInterface) => nInterface ?? [])
+		.filter(
+			(detail) =>
+				detail &&
+				detail.address &&
+				(detail.family === 'IPv4' ||
+					// @ts-expect-error Node 18.0 - 18.3 returns number
+					detail.family === 4)
+		)
+		.forEach((detail) => {
+			let host = detail.address.replace(
+				'127.0.0.1',
+				hostname === undefined || wildcardHosts.has(hostname) ? 'localhost' : hostname
+			);
+			// ipv6 host
+			if (host.includes(':')) {
+				host = `[${host}]`;
+			}
+			const url = `${protocol}://${host}:${port}${base ? base : ''}`;
+			if (detail.address.includes('127.0.0.1')) {
+				NetworkAddress.local.push(url);
+			} else {
+				NetworkAddress.network.push(url);
+			}
+		});
+	return NetworkAddress;
+}

packages/integrations/node/src/middleware.ts

@@ -0,0 +1,43 @@
+import { createAppHandler } from './serve-app.js';
+import type { RequestHandler } from "./types.js";
+import type { NodeApp } from "astro/app/node";
+
+/**
+ * Creates a middleware that can be used with Express, Connect, etc.
+ * 
+ * Similar to `createAppHandler` but can additionally be placed in the express
+ * chain as an error middleware.
+ * 
+ * https://expressjs.com/en/guide/using-middleware.html#middleware.error-handling
+ */
+export default function createMiddleware(
+    app: NodeApp,
+): RequestHandler {
+    const handler = createAppHandler(app)
+    const logger = app.getAdapterLogger()
+    // using spread args because express trips up if the function's
+    // stringified body includes req, res, next, locals directly
+    return async function (...args) {
+        // assume normal invocation at first
+        const [req, res, next, locals] = args;
+        // short circuit if it is an error invocation
+        if (req instanceof Error) {
+            const error = req;
+            if (next) {
+                return next(error);
+            } else {
+                throw error;
+            }
+        }
+		try {
+			await handler(req, res, next, locals);
+		} catch (err) {
+			logger.error(`Could not render ${req.url}`);
+			console.error(err);
+			if (!res.headersSent) {
+				res.writeHead(500, `Server error`);
+				res.end();
+			}
+		}
+    }
+}