Catch errors that occur within the stream in the Node adapter (#6935)

* Catch errors that occur within the stream in the Node adapter * Adding a changeset * Better error message on completely uncaught errors within the stream * Update test
withastro · May 1, 2023 · c405cef · c405cef
1 parent 649d709
commit c405cef
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 2 deletions.
diff --git a/.changeset/silent-years-burn.md b/.changeset/silent-years-burn.md
@@ -0,0 +1,5 @@
+---
+'@astrojs/node': patch
+---
+
+Catch errors that occur within the stream in the Node adapter
diff --git a/packages/integrations/node/src/middleware.ts b/packages/integrations/node/src/middleware.ts
@@ -51,8 +51,13 @@ async function writeWebResponse(app: NodeApp, res: ServerResponse, webResponse:
 
 	res.writeHead(status, Object.fromEntries(headers.entries()));
 	if (webResponse.body) {
-		for await (const chunk of responseIterator(webResponse) as unknown as Readable) {
-			res.write(chunk);
+		try {
+			for await (const chunk of responseIterator(webResponse) as unknown as Readable) {
+				res.write(chunk);
+			}
+		} catch(err: any) {
+			console.error(err?.stack || err?.message || String(err))
+			res.write('Internal server error');
 		}
 	}
 	res.end();

diff --git a/packages/integrations/node/test/errors.test.js b/packages/integrations/node/test/errors.test.js
@@ -0,0 +1,33 @@
+import nodejs from '../dist/index.js';
+import { loadFixture } from './test-utils.js';
+import { expect } from 'chai';
+import * as cheerio from 'cheerio';
+
+describe('Errors', () => {
+	let fixture;
+	before(async () => {
+		fixture = await loadFixture({
+			root: './fixtures/errors/',
+			output: 'server',
+			adapter: nodejs({ mode: 'standalone' }),
+		});
+		await fixture.build();
+	});
+	describe('Within the stream', async () => {
+		let devPreview;
+
+		before(async () => {
+			devPreview = await fixture.preview();
+		});
+		after(async () => {
+			await devPreview.stop();
+		});
+		it('when mode is standalone', async () => {
+			const res = await fixture.fetch('/in-stream');
+			const html = await res.text();
+			const $ = cheerio.load(html);
+
+			expect($('p').text().trim()).to.equal('Internal server error');
+		});
+	});
+});
diff --git a/packages/integrations/node/test/fixtures/errors/package.json b/packages/integrations/node/test/fixtures/errors/package.json
@@ -0,0 +1,9 @@
+{
+  "name": "@test/nodejs-errors",
+  "version": "0.0.0",
+  "private": true,
+  "dependencies": {
+    "astro": "workspace:*",
+    "@astrojs/node": "workspace:*"
+  }
+}
diff --git a/packages/integrations/node/test/fixtures/errors/src/pages/in-stream.astro b/packages/integrations/node/test/fixtures/errors/src/pages/in-stream.astro
@@ -0,0 +1,13 @@
+---
+---
+<html>
+	<head>
+		<title>One</title>
+	</head>
+	<body>
+		<h1>One</h1>
+		<p>
+			{Promise.reject('Error in the stream')}
+		</p>
+	</body>
+</html>
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml