From 5fc04b97ced00f5b1f06a5a542dbad2f9f3db796 Mon Sep 17 00:00:00 2001
From: lilnasy <69170106+lilnasy@users.noreply.github.com>
Date: Thu, 14 Mar 2024 20:57:43 +0000
Subject: [PATCH 1/8] fix(rendering): allow framework renders to be cancelled

---
 packages/astro/src/@types/astro.ts            |  4 +++
 packages/astro/src/core/render-context.ts     | 31 ++++++++++++-------
 .../src/runtime/server/render/astro/render.ts |  8 +++++
 .../src/runtime/server/render/component.ts    |  6 +++-
 4 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts
index 134e464d1324..06f7afadec50 100644
--- a/packages/astro/src/@types/astro.ts
+++ b/packages/astro/src/@types/astro.ts
@@ -2763,6 +2763,10 @@ export type SSRComponentMetadata = {
 };
 
 export interface SSRResult {
+	/**
+	 * A controller used to announce that the rendering the page has either failed with a non-recoverable error, or the client has disconnected.
+	 */
+	abortController: AbortController;
 	styles: Set<SSRElement>;
 	scripts: Set<SSRElement>;
 	links: Set<SSRElement>;
diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts
index d77ce6269d9f..0c9d0e0f83a4 100644
--- a/packages/astro/src/core/render-context.ts
+++ b/packages/astro/src/core/render-context.ts
@@ -89,15 +89,14 @@ export class RenderContext {
 		const apiContext = this.createAPIContext(props);
 		const { type } = routeData;
 
-		const lastNext =
-			type === 'endpoint'
-				? () => renderEndpoint(componentInstance as any, apiContext, serverLike, logger)
-				: type === 'redirect'
-					? () => renderRedirect(this)
-					: type === 'page'
-						? async () => {
+		const lastNext = async () => {
+			if (routeData.type === 'endpoint') return renderEndpoint(componentInstance as any, apiContext, serverLike, logger);
+			if (routeData.type === 'redirect') return renderRedirect(this);
+			if (routeData.type === 'page') {
 								const result = await this.createResult(componentInstance!);
-								const response = await renderPage(
+								let response: Response;
+								try {
+								response = await renderPage(
 									result,
 									componentInstance?.default as any,
 									props,
@@ -105,6 +104,12 @@ export class RenderContext {
 									streaming,
 									routeData
 								);
+								} catch (e) {
+									// If the instantiation of the RenderTemplate fails midway,
+									// we signal to ignore the results of existing renders and avoid kicking off more of them.
+									result.abortController.abort();
+									throw e;
+								}
 								// Signal to the i18n middleware to maybe act on this response
 								response.headers.set(ROUTE_TYPE_HEADER, 'page');
 								// Signal to the error-page-rerouting infra to let this response pass through to avoid loops
@@ -112,11 +117,10 @@ export class RenderContext {
 									response.headers.set(REROUTE_DIRECTIVE_HEADER, 'no');
 								}
 								return response;
-							}
-						: type === 'fallback'
-							? () =>
+				}
+				if (routeData.type === 'fallback') return (
 									new Response(null, { status: 500, headers: { [ROUTE_TYPE_HEADER]: 'fallback' } })
-							: () => {
+				)
 									throw new Error('Unknown type of route: ' + type);
 								};
 
@@ -196,10 +200,13 @@ export class RenderContext {
 			},
 		} satisfies AstroGlobal['response'];
 
+		const abortController = new AbortController;
+
 		// Create the result object that will be passed into the renderPage function.
 		// This object starts here as an empty shell (not yet the result) but then
 		// calling the render() function will populate the object with scripts, styles, etc.
 		const result: SSRResult = {
+			abortController,
 			clientDirectives,
 			inlinedScripts,
 			componentMetadata,
diff --git a/packages/astro/src/runtime/server/render/astro/render.ts b/packages/astro/src/runtime/server/render/astro/render.ts
index b8e35007b0c3..f82c116ff5b0 100644
--- a/packages/astro/src/runtime/server/render/astro/render.ts
+++ b/packages/astro/src/runtime/server/render/astro/render.ts
@@ -125,6 +125,11 @@ export async function renderToReadableStream(
 				}
 			})();
 		},
+		cancel() {
+			// If the client disconnects,
+			// we signal to ignore the results of existing renders and avoid kicking off more of them.
+			result.abortController.abort();
+		}
 	});
 }
 
@@ -244,6 +249,9 @@ export async function renderToAsyncIterable(
 		},
 		async return() {
 			cancelled = true;
+			// If the client disconnects,
+			// we signal to ignore the results of existing renders and avoid kicking off more of them.
+			result.abortController.abort();
 			return { done: true, value: undefined };
 		},
 	};
diff --git a/packages/astro/src/runtime/server/render/component.ts b/packages/astro/src/runtime/server/render/component.ts
index 4abbfeb07987..a7cb6ff1e92d 100644
--- a/packages/astro/src/runtime/server/render/component.ts
+++ b/packages/astro/src/runtime/server/render/component.ts
@@ -478,7 +478,11 @@ export async function renderComponent(
 		return renderAstroComponent(result, displayName, Component, props, slots);
 	}
 
-	return await renderFrameworkComponent(result, displayName, Component, props, slots);
+	return await renderFrameworkComponent(result, displayName, Component, props, slots)
+		.catch(e => {
+			if (result.abortController.signal.aborted) return { render() {} };
+			throw e;
+		});
 }
 
 function normalizeProps(props: Record<string, any>): Record<string, any> {

From b5c2c13998a45576f4d25851e976321d190fc5c5 Mon Sep 17 00:00:00 2001
From: lilnasy <69170106+lilnasy@users.noreply.github.com>
Date: Thu, 14 Mar 2024 20:58:26 +0000
Subject: [PATCH 2/8] add test

---
 .../test/fixtures/streaming/astro.config.mjs     |  5 +++++
 .../astro/test/fixtures/streaming/package.json   |  5 ++++-
 .../fixtures/streaming/src/components/react.tsx  |  8 ++++++++
 .../streaming/src/pages/multiple-errors.astro    |  7 +++++++
 packages/astro/test/streaming.test.js            | 16 +++++++++++-----
 pnpm-lock.yaml                                   |  9 +++++++++
 6 files changed, 44 insertions(+), 6 deletions(-)
 create mode 100644 packages/astro/test/fixtures/streaming/src/components/react.tsx
 create mode 100644 packages/astro/test/fixtures/streaming/src/pages/multiple-errors.astro

diff --git a/packages/astro/test/fixtures/streaming/astro.config.mjs b/packages/astro/test/fixtures/streaming/astro.config.mjs
index e69de29bb2d1..0773be443504 100644
--- a/packages/astro/test/fixtures/streaming/astro.config.mjs
+++ b/packages/astro/test/fixtures/streaming/astro.config.mjs
@@ -0,0 +1,5 @@
+import react from "@astrojs/react"
+
+export default {
+    integrations: [ react() ]
+}
diff --git a/packages/astro/test/fixtures/streaming/package.json b/packages/astro/test/fixtures/streaming/package.json
index a27a51b6dcf4..44c2626d700c 100644
--- a/packages/astro/test/fixtures/streaming/package.json
+++ b/packages/astro/test/fixtures/streaming/package.json
@@ -6,6 +6,9 @@
     "dev": "astro dev"
   },
   "dependencies": {
-    "astro": "workspace:*"
+    "astro": "workspace:*",
+    "@astrojs/react": "workspace:*",
+    "react": "^18.2.0",
+    "react-dom": "^18.2.0"
   }
 }
diff --git a/packages/astro/test/fixtures/streaming/src/components/react.tsx b/packages/astro/test/fixtures/streaming/src/components/react.tsx
new file mode 100644
index 000000000000..7d83e01ff29c
--- /dev/null
+++ b/packages/astro/test/fixtures/streaming/src/components/react.tsx
@@ -0,0 +1,8 @@
+export default function ReactComp({ foo }: { foo: { bar: { baz: string[] } } }) {
+    return (
+      <div>
+        React Comp
+        {foo.bar.baz.length}
+      </div>
+    );
+}
diff --git a/packages/astro/test/fixtures/streaming/src/pages/multiple-errors.astro b/packages/astro/test/fixtures/streaming/src/pages/multiple-errors.astro
new file mode 100644
index 000000000000..7d922ef0d809
--- /dev/null
+++ b/packages/astro/test/fixtures/streaming/src/pages/multiple-errors.astro
@@ -0,0 +1,7 @@
+---
+import ReactComp from '../components/react.tsx';
+
+const foo = { bar: null } as any;
+---
+<ReactComp foo={foo} />
+{foo.bar.baz.length > 0 && <div/>}
diff --git a/packages/astro/test/streaming.test.js b/packages/astro/test/streaming.test.js
index 1cfe7f42f1c1..6a0bdcf4e346 100644
--- a/packages/astro/test/streaming.test.js
+++ b/packages/astro/test/streaming.test.js
@@ -2,11 +2,9 @@ import assert from 'node:assert/strict';
 import { after, before, describe, it } from 'node:test';
 import * as cheerio from 'cheerio';
 import testAdapter from './test-adapter.js';
-import { isWindows, loadFixture, streamAsyncIterator } from './test-utils.js';
+import { loadFixture, streamAsyncIterator } from './test-utils.js';
 
 describe('Streaming', () => {
-	if (isWindows) return;
-
 	/** @type {import('./test-utils').Fixture} */
 	let fixture;
 
@@ -79,12 +77,20 @@ describe('Streaming', () => {
 			}
 			assert.equal(chunks.length > 1, true);
 		});
+
+		// if the offshoot promise goes unhandled, this test will pass immediately but fail the test suite 
+		it('Stays alive on failed component renders initiated by failed render templates', async () => {
+			const app = await fixture.loadTestAdapterApp();
+			const request = new Request('http://example.com/multiple-errors');
+			const response = await app.render(request);
+			assert.equal(response.status, 500);
+			const text = await response.text();
+			assert.equal(text, '');
+		});
 	});
 });
 
 describe('Streaming disabled', () => {
-	if (isWindows) return;
-
 	/** @type {import('./test-utils').Fixture} */
 	let fixture;
 
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
index 9993ab1ce527..32ec82766336 100644
--- a/pnpm-lock.yaml
+++ b/pnpm-lock.yaml
@@ -3637,9 +3637,18 @@ importers:
 
   packages/astro/test/fixtures/streaming:
     dependencies:
+      '@astrojs/react':
+        specifier: workspace:*
+        version: link:../../../../integrations/react
       astro:
         specifier: workspace:*
         version: link:../../..
+      react:
+        specifier: ^18.2.0
+        version: 18.2.0
+      react-dom:
+        specifier: ^18.2.0
+        version: 18.2.0(react@18.2.0)
 
   packages/astro/test/fixtures/svelte-component:
     dependencies:

From 97cc5b04298ba3ced066923248415ce7bbe4efb5 Mon Sep 17 00:00:00 2001
From: lilnasy <69170106+lilnasy@users.noreply.github.com>
Date: Thu, 14 Mar 2024 20:59:46 +0000
Subject: [PATCH 3/8] add changeset

---
 .changeset/slow-rabbits-care.md | 5 +++++
 1 file changed, 5 insertions(+)
 create mode 100644 .changeset/slow-rabbits-care.md

diff --git a/.changeset/slow-rabbits-care.md b/.changeset/slow-rabbits-care.md
new file mode 100644
index 000000000000..c7e64bd3368d
--- /dev/null
+++ b/.changeset/slow-rabbits-care.md
@@ -0,0 +1,5 @@
+---
+"astro": patch
+---
+
+Fixes an issue where multiple rendering errors resulted in a crash of the SSR app server.

From 68afd5ea5769eb6ceb4afae097fcb4b6fdd7af1c Mon Sep 17 00:00:00 2001
From: lilnasy <69170106+lilnasy@users.noreply.github.com>
Date: Thu, 14 Mar 2024 21:08:51 +0000
Subject: [PATCH 4/8] `AbortController` -> `boolean`

---
 packages/astro/src/@types/astro.ts                    |  4 ++--
 packages/astro/src/core/render-context.ts             |  8 +++-----
 .../astro/src/runtime/server/render/astro/render.ts   | 11 ++++-------
 packages/astro/src/runtime/server/render/component.ts |  2 +-
 4 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts
index 06f7afadec50..4ab2d50480a9 100644
--- a/packages/astro/src/@types/astro.ts
+++ b/packages/astro/src/@types/astro.ts
@@ -2764,9 +2764,9 @@ export type SSRComponentMetadata = {
 
 export interface SSRResult {
 	/**
-	 * A controller used to announce that the rendering the page has either failed with a non-recoverable error, or the client has disconnected.
+	 * Whether the page has either failed with a non-recoverable error, or the client has disconnected.
 	 */
-	abortController: AbortController;
+	cancelled: boolean;
 	styles: Set<SSRElement>;
 	scripts: Set<SSRElement>;
 	links: Set<SSRElement>;
diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts
index 0c9d0e0f83a4..dfce33cb95d3 100644
--- a/packages/astro/src/core/render-context.ts
+++ b/packages/astro/src/core/render-context.ts
@@ -106,8 +106,8 @@ export class RenderContext {
 								);
 								} catch (e) {
 									// If the instantiation of the RenderTemplate fails midway,
-									// we signal to ignore the results of existing renders and avoid kicking off more of them.
-									result.abortController.abort();
+									// we signal to the rest of the internals that we can ignore the results of existing renders and avoid kicking off more of them.
+									result.cancelled = true;
 									throw e;
 								}
 								// Signal to the i18n middleware to maybe act on this response
@@ -200,13 +200,11 @@ export class RenderContext {
 			},
 		} satisfies AstroGlobal['response'];
 
-		const abortController = new AbortController;
-
 		// Create the result object that will be passed into the renderPage function.
 		// This object starts here as an empty shell (not yet the result) but then
 		// calling the render() function will populate the object with scripts, styles, etc.
 		const result: SSRResult = {
-			abortController,
+			cancelled: false,
 			clientDirectives,
 			inlinedScripts,
 			componentMetadata,
diff --git a/packages/astro/src/runtime/server/render/astro/render.ts b/packages/astro/src/runtime/server/render/astro/render.ts
index f82c116ff5b0..da771397edc0 100644
--- a/packages/astro/src/runtime/server/render/astro/render.ts
+++ b/packages/astro/src/runtime/server/render/astro/render.ts
@@ -128,7 +128,7 @@ export async function renderToReadableStream(
 		cancel() {
 			// If the client disconnects,
 			// we signal to ignore the results of existing renders and avoid kicking off more of them.
-			result.abortController.abort();
+			result.cancelled = true;
 		}
 	});
 }
@@ -206,13 +206,11 @@ export async function renderToAsyncIterable(
 	// The `next` is an object `{ promise, resolve, reject }` that we use to wait
 	// for chunks to be pushed into the buffer.
 	let next = promiseWithResolvers<void>();
-	// keep track of whether the client connection is still interested in the response.
-	let cancelled = false;
 	const buffer: Uint8Array[] = []; // []Uint8Array
 
 	const iterator: AsyncIterator<Uint8Array> = {
 		async next() {
-			if (cancelled) return { done: true, value: undefined };
+			if (result.cancelled) return { done: true, value: undefined };
 
 			await next.promise;
 
@@ -248,10 +246,9 @@ export async function renderToAsyncIterable(
 			return returnValue;
 		},
 		async return() {
-			cancelled = true;
 			// If the client disconnects,
-			// we signal to ignore the results of existing renders and avoid kicking off more of them.
-			result.abortController.abort();
+			// we signal to the rest of the internals to ignore the results of existing renders and avoid kicking off more of them.
+			result.cancelled = true;
 			return { done: true, value: undefined };
 		},
 	};
diff --git a/packages/astro/src/runtime/server/render/component.ts b/packages/astro/src/runtime/server/render/component.ts
index a7cb6ff1e92d..36dc3c39e23a 100644
--- a/packages/astro/src/runtime/server/render/component.ts
+++ b/packages/astro/src/runtime/server/render/component.ts
@@ -480,7 +480,7 @@ export async function renderComponent(
 
 	return await renderFrameworkComponent(result, displayName, Component, props, slots)
 		.catch(e => {
-			if (result.abortController.signal.aborted) return { render() {} };
+			if (result.cancelled) return { render() {} };
 			throw e;
 		});
 }

From 74e48f330fafc135904c1bccc20c962c4ef7871c Mon Sep 17 00:00:00 2001
From: lilnasy <69170106+lilnasy@users.noreply.github.com>
Date: Fri, 15 Mar 2024 06:41:37 +0000
Subject: [PATCH 5/8] update comments

---
 packages/astro/src/@types/astro.ts        | 4 ++--
 packages/astro/src/core/render-context.ts | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts
index 4ab2d50480a9..ba2588b4a0ba 100644
--- a/packages/astro/src/@types/astro.ts
+++ b/packages/astro/src/@types/astro.ts
@@ -2764,8 +2764,8 @@ export type SSRComponentMetadata = {
 
 export interface SSRResult {
 	/**
-	 * Whether the page has either failed with a non-recoverable error, or the client has disconnected.
-	 */
+	 * Whether the page has failed with a non-recoverable error, or the client disconnected.
+	 */	
 	cancelled: boolean;
 	styles: Set<SSRElement>;
 	scripts: Set<SSRElement>;
diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts
index dfce33cb95d3..7a91d8d4b41d 100644
--- a/packages/astro/src/core/render-context.ts
+++ b/packages/astro/src/core/render-context.ts
@@ -105,7 +105,7 @@ export class RenderContext {
 									routeData
 								);
 								} catch (e) {
-									// If the instantiation of the RenderTemplate fails midway,
+									// If there is an error in the page's frontmatter or instantiation of the RenderTemplate fails midway,
 									// we signal to the rest of the internals that we can ignore the results of existing renders and avoid kicking off more of them.
 									result.cancelled = true;
 									throw e;

From 637c0ef668421a0a02362c3d5984d6aae2ccd652 Mon Sep 17 00:00:00 2001
From: lilnasy <69170106+lilnasy@users.noreply.github.com>
Date: Fri, 15 Mar 2024 13:46:45 +0000
Subject: [PATCH 6/8] handle cancellation for fragments and html components

---
 .../src/runtime/server/render/component.ts    | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/packages/astro/src/runtime/server/render/component.ts b/packages/astro/src/runtime/server/render/component.ts
index 36dc3c39e23a..e4cc737acce0 100644
--- a/packages/astro/src/runtime/server/render/component.ts
+++ b/packages/astro/src/runtime/server/render/component.ts
@@ -459,11 +459,13 @@ export async function renderComponent(
 	slots: any = {}
 ): Promise<RenderInstance> {
 	if (isPromise(Component)) {
-		Component = await Component;
+		Component = await Component
+			.catch(handleCancellation);
 	}
 
 	if (isFragmentComponent(Component)) {
-		return await renderFragmentComponent(result, slots);
+		return await renderFragmentComponent(result, slots)
+			.catch(handleCancellation);
 	}
 
 	// Ensure directives (`class:list`) are processed
@@ -471,7 +473,8 @@ export async function renderComponent(
 
 	// .html components
 	if (isHTMLComponent(Component)) {
-		return await renderHTMLComponent(result, Component, props, slots);
+		return await renderHTMLComponent(result, Component, props, slots)
+			.catch(handleCancellation);
 	}
 
 	if (isAstroComponentFactory(Component)) {
@@ -479,10 +482,12 @@ export async function renderComponent(
 	}
 
 	return await renderFrameworkComponent(result, displayName, Component, props, slots)
-		.catch(e => {
-			if (result.cancelled) return { render() {} };
-			throw e;
-		});
+		.catch(handleCancellation);
+
+	function handleCancellation(e: unknown) {
+		if (result.cancelled) return { render() {} };
+		throw e;
+	}
 }
 
 function normalizeProps(props: Record<string, any>): Record<string, any> {

From cba64a1c56a51cce14418668244e5cdcb306295d Mon Sep 17 00:00:00 2001
From: lilnasy <69170106+lilnasy@users.noreply.github.com>
Date: Fri, 15 Mar 2024 13:51:59 +0000
Subject: [PATCH 7/8] if-else -> switch-case

---
 packages/astro/src/core/render-context.ts | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts
index 7a91d8d4b41d..ce277083564d 100644
--- a/packages/astro/src/core/render-context.ts
+++ b/packages/astro/src/core/render-context.ts
@@ -87,12 +87,12 @@ export class RenderContext {
 			serverLike,
 		});
 		const apiContext = this.createAPIContext(props);
-		const { type } = routeData;
 
 		const lastNext = async () => {
-			if (routeData.type === 'endpoint') return renderEndpoint(componentInstance as any, apiContext, serverLike, logger);
-			if (routeData.type === 'redirect') return renderRedirect(this);
-			if (routeData.type === 'page') {
+			switch (routeData.type) {
+				case 'endpoint': return renderEndpoint(componentInstance as any, apiContext, serverLike, logger);
+				case 'redirect': return renderRedirect(this);
+				case 'page': {
 								const result = await this.createResult(componentInstance!);
 								let response: Response;
 								try {
@@ -117,12 +117,14 @@ export class RenderContext {
 									response.headers.set(REROUTE_DIRECTIVE_HEADER, 'no');
 								}
 								return response;
+					}
+					case 'fallback': {
+									return (
+										new Response(null, { status: 500, headers: { [ROUTE_TYPE_HEADER]: 'fallback' } })
+									)
+					}
 				}
-				if (routeData.type === 'fallback') return (
-									new Response(null, { status: 500, headers: { [ROUTE_TYPE_HEADER]: 'fallback' } })
-				)
-									throw new Error('Unknown type of route: ' + type);
-								};
+			}
 
 		const response = await callMiddleware(middleware, apiContext, lastNext);
 		if (response.headers.get(ROUTE_TYPE_HEADER)) {

From dd66cdf885f5c057bbf8fbd9faf69ffac93822b2 Mon Sep 17 00:00:00 2001
From: lilnasy <69170106+lilnasy@users.noreply.github.com>
Date: Fri, 15 Mar 2024 13:53:02 +0000
Subject: [PATCH 8/8] de-indent

---
 packages/astro/src/core/render-context.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts
index ce277083564d..7460ad751673 100644
--- a/packages/astro/src/core/render-context.ts
+++ b/packages/astro/src/core/render-context.ts
@@ -120,7 +120,7 @@ export class RenderContext {
 					}
 					case 'fallback': {
 									return (
-										new Response(null, { status: 500, headers: { [ROUTE_TYPE_HEADER]: 'fallback' } })
+									new Response(null, { status: 500, headers: { [ROUTE_TYPE_HEADER]: 'fallback' } })
 									)
 					}
 				}