From 83f9105cd50e2756d02ca2be73ab84f9d582d3f8 Mon Sep 17 00:00:00 2001
From: Arsh <69170106+lilnasy@users.noreply.github.com>
Date: Mon, 18 Mar 2024 19:44:46 +0530
Subject: [PATCH] fix(node): handle offshoot promise rejections (#10454)

* fix(node): handle offshoot promise rejections

* add test

* add changeset

* Update packages/integrations/node/test/errors.test.js
---
 .changeset/sharp-flowers-ring.md              |  5 ++++
 packages/integrations/node/src/serve-app.ts   | 19 ++++++++++--
 .../integrations/node/test/errors.test.js     | 30 +++++++++++++++++--
 .../pages/offshoot-promise-rejection.astro    |  2 ++
 4 files changed, 51 insertions(+), 5 deletions(-)
 create mode 100644 .changeset/sharp-flowers-ring.md
 create mode 100644 packages/integrations/node/test/fixtures/errors/src/pages/offshoot-promise-rejection.astro

diff --git a/.changeset/sharp-flowers-ring.md b/.changeset/sharp-flowers-ring.md
new file mode 100644
index 000000000000..c1a5891f3577
--- /dev/null
+++ b/.changeset/sharp-flowers-ring.md
@@ -0,0 +1,5 @@
+---
+"@astrojs/node": patch
+---
+
+Prevents crashes caused by rejections of offshoot promises.
diff --git a/packages/integrations/node/src/serve-app.ts b/packages/integrations/node/src/serve-app.ts
index a9840b72148c..ac5af3addebe 100644
--- a/packages/integrations/node/src/serve-app.ts
+++ b/packages/integrations/node/src/serve-app.ts
@@ -1,3 +1,4 @@
+import { AsyncLocalStorage } from 'node:async_hooks';
 import { NodeApp } from 'astro/app/node';
 import type { RequestHandler } from './types.js';
 
@@ -7,8 +8,20 @@ import type { RequestHandler } from './types.js';
  * Intended to be used in both standalone and middleware mode.
  */
 export function createAppHandler(app: NodeApp): RequestHandler {
+	/**
+	 * Keep track of the current request path using AsyncLocalStorage.
+	 * Used to log unhandled rejections with a helpful message.
+	 */
+	const als = new AsyncLocalStorage<string>();
+	const logger = app.getAdapterLogger();
+	process.on('unhandledRejection', reason => {
+		const requestUrl = als.getStore();
+		logger.error(`Unhandled rejection while rendering ${requestUrl}`);
+		console.error(reason);
+	});
+
 	return async (req, res, next, locals) => {
-		let request;
+		let request: Request;
 		try {
 			request = NodeApp.createRequest(req);
 		} catch (err) {
@@ -19,11 +32,11 @@ export function createAppHandler(app: NodeApp): RequestHandler {
 
 		const routeData = app.match(request);
 		if (routeData) {
-			const response = await app.render(request, {
+			const response = await als.run(request.url, () => app.render(request, {
 				addCookieHeader: true,
 				locals,
 				routeData,
-			});
+			}));
 			await NodeApp.writeResponse(response, res);
 		} else if (next) {
 			return next();
diff --git a/packages/integrations/node/test/errors.test.js b/packages/integrations/node/test/errors.test.js
index 95bb1be85466..1e435ddd7d0c 100644
--- a/packages/integrations/node/test/errors.test.js
+++ b/packages/integrations/node/test/errors.test.js
@@ -1,11 +1,16 @@
+import { spawn } from 'node:child_process';
+import { Worker } from 'node:worker_threads';
 import assert from 'node:assert/strict';
 import { after, before, describe, it } from 'node:test';
 import * as cheerio from 'cheerio';
 import nodejs from '../dist/index.js';
 import { loadFixture } from './test-utils.js';
+import { fileURLToPath } from 'node:url';
 
 describe('Errors', () => {
+	/** @type {import('./test-utils.js').Fixture} */
 	let fixture;
+
 	before(async () => {
 		fixture = await loadFixture({
 			root: './fixtures/errors/',
@@ -17,10 +22,31 @@ describe('Errors', () => {
 	let devPreview;
 
 	before(async () => {
-		devPreview = await fixture.preview();
+		// The two tests that need the server to run are skipped
+		// devPreview = await fixture.preview();
 	});
 	after(async () => {
-		await devPreview.stop();
+		await devPreview?.stop();
+	});
+
+	it('stays alive after offshoot promise rejections', async () => {
+		// this test needs to happen in a worker because node test runner adds a listener for unhandled rejections in the main thread
+		const worker = new Worker('./test/fixtures/errors/dist/server/entry.mjs', {
+			type: 'module',
+			env: { ASTRO_NODE_LOGGING: 'enabled' }
+		});
+		
+		await new Promise((resolve, reject) => {
+			worker.stdout.on('data', data => {
+				setTimeout(() => reject("Server took too long to start"), 1000);
+				if (data.toString().includes('Server listening on http://localhost:4321')) resolve();
+			});
+		});
+		
+		await fetch("http://localhost:4321/offshoot-promise-rejection");
+
+		// if there was a crash, it becomes an error here
+		await worker.terminate();
 	});
 
 	it(
diff --git a/packages/integrations/node/test/fixtures/errors/src/pages/offshoot-promise-rejection.astro b/packages/integrations/node/test/fixtures/errors/src/pages/offshoot-promise-rejection.astro
new file mode 100644
index 000000000000..be702d5ef730
--- /dev/null
+++ b/packages/integrations/node/test/fixtures/errors/src/pages/offshoot-promise-rejection.astro
@@ -0,0 +1,2 @@
+{new Promise(async _ => (await {}, Astro.props.undefined.alsoAPropertyOfUndefined))}
+{Astro.props.undefined.propertyOfUndefined}
\ No newline at end of file