Prevent mdx server hangs on JSX checking (#5330)

* Prevent mdx server hangs on JSX checking

* Adding a changeset

* Try a different approach to skips

.changeset/hot-parrots-exist.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Prevent jsx throws from hanging server

packages/astro/src/jsx-runtime/index.ts

@@ -7,7 +7,7 @@ export interface AstroVNode {
 	[Renderer]: string;
 	[AstroJSX]: boolean;
 	type: string | ((...args: any) => any);
-	props: Record<string, any>;
+	props: Record<string | symbol, any>;
 }
 
 const toSlotName = (slotAttr: string) => slotAttr;

packages/astro/src/runtime/server/hydration.ts

@@ -23,14 +23,14 @@ export interface HydrationMetadata {
 interface ExtractedProps {
 	isPage: boolean;
 	hydration: HydrationMetadata | null;
-	props: Record<string | number, any>;
+	props: Record<string | number | symbol, any>;
 }
 
 // Used to extract the directives, aka `client:load` information about a component.
 // Finds these special props and removes them from what gets passed into the component.
 export function extractDirectives(
 	displayName: string,
-	inputProps: Record<string | number, any>
+	inputProps: Record<string | number | symbol, any>
 ): ExtractedProps {
 	let extracted: ExtractedProps = {
 		isPage: false,
@@ -105,6 +105,9 @@ export function extractDirectives(
 			extracted.props[key] = value;
 		}
 	}
+	for(const sym of Object.getOwnPropertySymbols(inputProps)) {
+		extracted.props[sym] = inputProps[sym];
+	}
 
 	return extracted;
 }

packages/astro/src/runtime/server/jsx.ts

@@ -1,6 +1,6 @@
 /* eslint-disable no-console */
 import { SSRResult } from '../../@types/astro.js';
-import { AstroJSX, isVNode } from '../../jsx-runtime/index.js';
+import { AstroJSX, AstroVNode, isVNode } from '../../jsx-runtime/index.js';
 import {
 	escapeHTML,
 	HTMLString,
@@ -15,7 +15,26 @@ import type { ComponentIterable } from './render/component';
 
 const ClientOnlyPlaceholder = 'astro-client-only';
 
-const skipAstroJSXCheck = new WeakSet();
+class Skip {
+	count: number;
+	constructor(public vnode: AstroVNode) {
+		this.count = 0;
+	}
+
+	increment() {
+		this.count++;
+	}
+
+	haveNoTried() {
+		return this.count === 0;
+	}
+	
+	isCompleted() {
+		return this.count > 2;
+	}
+	static symbol = Symbol('astro:jsx:skip');
+}
+
 let originalConsoleError: any;
 let consoleFilterRefs = 0;
 
@@ -37,6 +56,19 @@ export async function renderJSX(result: SSRResult, vnode: any): Promise<any> {
 				(await Promise.all(vnode.map((v: any) => renderJSX(result, v)))).join('')
 			);
 	}
+
+	// Extract the skip from the props, if we've already attempted a previous render
+	let skip: Skip;
+	if(vnode.props[Skip.symbol]) {
+		skip = vnode.props[Skip.symbol];
+	} else {
+		skip = new Skip(vnode);
+	}
+
+	return renderJSXVNode(result, vnode, skip);
+}
+
+async function renderJSXVNode(result: SSRResult, vnode: AstroVNode, skip: Skip): Promise<any> {
 	if (isVNode(vnode)) {
 		switch (true) {
 			case !vnode.type: {
@@ -68,26 +100,37 @@ Did you forget to import the component or is it possible there is a typo?`);
 
 		if (vnode.type) {
 			if (typeof vnode.type === 'function' && (vnode.type as any)['astro:renderer']) {
-				skipAstroJSXCheck.add(vnode.type);
+				skip.increment();
 			}
 			if (typeof vnode.type === 'function' && vnode.props['server:root']) {
 				const output = await vnode.type(vnode.props ?? {});
 				return await renderJSX(result, output);
 			}
-			if (typeof vnode.type === 'function' && !skipAstroJSXCheck.has(vnode.type)) {
+			if (typeof vnode.type === 'function') {
+				if(skip.haveNoTried() || skip.isCompleted()) {
 					useConsoleFilter();
 					try {
 						const output = await vnode.type(vnode.props ?? {});
+						let renderResult: any;
 						if (output && output[AstroJSX]) {
-						return await renderJSX(result, output);
+							renderResult = await renderJSXVNode(result, output, skip);
+							return renderResult;
 						} else if (!output) {
-						return await renderJSX(result, output);
+							renderResult = await renderJSXVNode(result, output, skip);
+							return renderResult;
 						}
-				} catch (e) {
-					skipAstroJSXCheck.add(vnode.type);
+						
+					} catch (e: unknown) {
+						if(skip.isCompleted()) {
+							throw e;
+						}
+						skip.increment();
 					} finally {
 						finishUsingConsoleFilter();
 					}
+				} else {
+					skip.increment();
+				}
 			}
 
 			const { children = null, ...props } = vnode.props ?? {};
@@ -128,6 +171,7 @@ Did you forget to import the component or is it possible there is a typo?`);
 			}
 			await Promise.all(slotPromises);
 
+			props[Skip.symbol] = skip;
 			let output: ComponentIterable;
 			if (vnode.type === ClientOnlyPlaceholder && vnode.props['client:only']) {
 				output = await renderComponent(

packages/astro/test/units/render/jsx.test.js

@@ -1,6 +1,6 @@
 import { expect } from 'chai';
 
-import { createComponent, render, renderSlot } from '../../../dist/runtime/server/index.js';
+import { createComponent, render, renderComponent, renderSlot } from '../../../dist/runtime/server/index.js';
 import { jsx } from '../../../dist/jsx-runtime/index.js';
 import {
 	createBasicEnvironment,
@@ -90,5 +90,25 @@ describe('core/render', () => {
 				'<main><div><p class="n">works</p></div><div><p class="p">works</p></div></main>'
 			);
 		});
+
+		it('Errors in JSX components are raised', async () => {
+			const Component = createAstroJSXComponent(() => {
+				throw new Error('uh oh');
+			});
+
+			const Page = createComponent((result, _props) => {
+				return render`<div>${renderComponent(result, 'Component', Component, {})}</div>`;
+			});
+
+			const ctx = createRenderContext({ request: new Request('http://example.com/') });
+			const response = await renderPage(createAstroModule(Page), ctx, env);
+
+			try {
+				await response.text();
+				expect(false).to.equal(true, 'should not have been successful');
+			} catch(err) {
+				expect(err.message).to.equal('uh oh');
+			}
+		});
 	});
 });