Try a counter approach

Keep track of error so we can rethrow later
Adding a changeset
2022-10-10 08:28:55 -04:00 · 2022-10-07 16:09:53 -04:00 · 2022-10-07 15:48:58 -04:00 · 2022-10-07 15:48:17 -04:00
9 changed files with 120 additions and 15 deletions
--- a/.changeset/hungry-tomatoes-cough.md
+++ b/.changeset/hungry-tomatoes-cough.md
@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Prevent jsx throws from hanging server
--- a/packages/astro/src/runtime/server/jsx.ts
+++ b/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,24 @@ import type { ComponentIterable } from './render/component';

 const ClientOnlyPlaceholder = 'astro-client-only';

-const skipAstroJSXCheck = new WeakSet();
+const skipAstroJSXCheck = new WeakMap<() => any, number>();
+function addSkip(vnode: AstroVNode) {
+	if(typeof vnode.type === 'function') {
+		skipAstroJSXCheck.set(vnode.type, skipCount(vnode) + 1);
+	}
+}
+function skipCount(vnode: AstroVNode): number {
+	if(typeof vnode.type === 'function') {
+		return  skipAstroJSXCheck.get(vnode.type) || 0;
+	}
+	return NaN;
+}
+function deleteSkips(vnode: AstroVNode) {
+	if(typeof vnode.type === 'function') {
+		skipAstroJSXCheck.delete(vnode.type);
+	}
+}
+
 let originalConsoleError: any;
 let consoleFilterRefs = 0;

@ -68,26 +85,36 @@ 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);
+				addSkip(vnode);
 			}
 			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(skipCount(vnode) === 0 || skipCount(vnode) > 2) {
 					useConsoleFilter();
 					try {
 						const output = await vnode.type(vnode.props ?? {});
+						let renderResult: any;
 						if (output && output[AstroJSX]) {
-						return await renderJSX(result, output);
+							renderResult = await renderJSX(result, output);
 						} else if (!output) {
-						return await renderJSX(result, output);
+							renderResult = await renderJSX(result, output);
 						}
-				} catch (e) {
-					skipAstroJSXCheck.add(vnode.type);
+						deleteSkips(vnode);
+						return renderResult;
+					} catch (e: any) {
+						if(skipCount(vnode) > 2) {
+							throw e;
+						}
+						addSkip(vnode);
 					} finally {
 						finishUsingConsoleFilter();
 					}
+				} else {
+					addSkip(vnode);
+				}
 			}

 			const { children = null, ...props } = vnode.props ?? {};
@ -151,8 +178,10 @@ Did you forget to import the component or is it possible there is a typo?`);
 				for await (const chunk of output) {
 					parts.append(chunk, result);
 				}
+				deleteSkips(vnode);
 				return markHTMLString(parts.toString());
 			} else {
+				deleteSkips(vnode);
 				return markHTMLString(output);
 			}
 		}
--- a/packages/integrations/mdx/test/fixtures/mdx-throw-error/astro.config.mjs
+++ b/packages/integrations/mdx/test/fixtures/mdx-throw-error/astro.config.mjs
@ -0,0 +1,5 @@
+import mdx from '@astrojs/mdx';
+
+export default {
+	integrations: [mdx()]
+}
--- a/packages/integrations/mdx/test/fixtures/mdx-throw-error/package.json
+++ b/packages/integrations/mdx/test/fixtures/mdx-throw-error/package.json
@ -0,0 +1,7 @@
+{
+  "name": "@test/mdx-throw-error",
+  "dependencies": {
+    "@astrojs/mdx": "workspace:*",
+    "astro": "workspace:*"
+  }
+}
--- a/packages/integrations/mdx/test/fixtures/mdx-throw-error/src/components/component.mdx
+++ b/packages/integrations/mdx/test/fixtures/mdx-throw-error/src/components/component.mdx
@ -0,0 +1,5 @@
+import { throwError } from '../js/throwerror';
+
+This will probably result in a stack overflow. :(
+
+<pre>{ throwError() }</pre>
--- a/packages/integrations/mdx/test/fixtures/mdx-throw-error/src/js/throwerror.ts
+++ b/packages/integrations/mdx/test/fixtures/mdx-throw-error/src/js/throwerror.ts
@ -0,0 +1,5 @@
+export function throwError() {
+	console.log(`I'm going to throw an error. The server will just keep spinning until it runs out of memory...`);
+
+	throw new Error('Oh no');
+}
--- a/packages/integrations/mdx/test/fixtures/mdx-throw-error/src/pages/index.astro
+++ b/packages/integrations/mdx/test/fixtures/mdx-throw-error/src/pages/index.astro
@ -0,0 +1,16 @@
+---
+
+import Component from '../components/component.mdx';
+---
+<html>
+<head>
+	<title>Testing</title>
+</head>
+<body>
+	<main>
+		<h1>Astro MDX Errors</h1>
+
+		<Component />
+	</main>
+</body>
+</html>
--- a/packages/integrations/mdx/test/mdx-throw-errors.test.js
+++ b/packages/integrations/mdx/test/mdx-throw-errors.test.js
@ -0,0 +1,25 @@
+import mdx from '@astrojs/mdx';
+
+import { expect } from 'chai';
+import { loadFixture } from '../../../astro/test/test-utils.js';
+
+describe('MDX errors', () => {
+	/** @type {import('../../../astro/test/test-utils.js').Fixture} */
+	let fixture;
+
+	before(async () => {
+		fixture = await loadFixture({
+			root: new URL('./fixtures/mdx-throw-error/', import.meta.url),
+			integrations: [mdx()],
+		});
+	});
+
+	it('throws error during the build (does not lock up)', async () => {
+		try {
+			await fixture.build();
+			expect(false).to.equal(true);
+		} catch(err) {
+			expect(err.message).to.equal('Oh no');
+		}
+	});
+});
--- a/pnpm-lock.yaml
+++ b/pnpm-lock.yaml
@ -2821,6 +2821,14 @@ importers:
      react: 18.2.0
      react-dom: 18.2.0_react@18.2.0

+  packages/integrations/mdx/test/fixtures/mdx-throw-error:
+    specifiers:
+      '@astrojs/mdx': workspace:*
+      astro: workspace:*
+    dependencies:
+      '@astrojs/mdx': link:../../..
+      astro: link:../../../../../astro
+
  packages/integrations/mdx/test/fixtures/mdx-vite-env-vars:
    specifiers:
      '@astrojs/mdx': workspace:*
Author	SHA1	Message	Date
Matthew Phillips	e540b6b68a	Try a counter approach	2022-10-10 08:28:55 -04:00
Matthew Phillips	410cfe306c	Keep track of error so we can rethrow later	2022-10-07 16:09:53 -04:00
Matthew Phillips	f67f18af7a	Adding a changeset	2022-10-07 15:48:58 -04:00
Matthew Phillips	25643a0beb	Prevent server hang in MDX when code throws	2022-10-07 15:48:17 -04:00