fix(NodeApp): end with "Internal server error" on mid-stream error (#…

…9908) * fix(NodeApp): end with "Internal server error" on mid-stream error * add changeset * add test * Apply suggestions from code review --------- Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>
withastro · Feb 1, 2024 · 2f6d1fa · 2f6d1fa
1 parent 440bdff
commit 2f6d1fa
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 4 deletions.
diff --git a/.changeset/clever-roses-sleep.md b/.changeset/clever-roses-sleep.md
@@ -0,0 +1,5 @@
+---
+"astro": patch
+---
+
+Improves http behavior relating to errors encountered while streaming a response.
diff --git a/packages/astro/src/core/app/node.ts b/packages/astro/src/core/app/node.ts
@@ -118,12 +118,12 @@ export class NodeApp extends App {
 					destination.write(result.value);
 					result = await reader.read();
 				}
+				destination.end();
 				// the error will be logged by the "on end" callback above
 			} catch {
-				destination.write('Internal server error');
+				destination.end('Internal server error');
 			}
 		}
-		destination.end();
 	}
 }
 

diff --git a/packages/integrations/node/test/errors.test.js b/packages/integrations/node/test/errors.test.js
@@ -1,4 +1,4 @@
-import * as assert from 'node:assert/strict';
+import assert from 'node:assert/strict';
 import { describe, it, before, after } from 'node:test';
 import nodejs from '../dist/index.js';
 import { loadFixture } from './test-utils.js';
@@ -22,11 +22,26 @@ describe('Errors', () => {
 	after(async () => {
 		await devPreview.stop();
 	});
-	it('when mode is standalone', async () => {
+
+	it('rejected promise in template', async () => {
 		const res = await fixture.fetch('/in-stream');
 		const html = await res.text();
 		const $ = cheerio.load(html);
 
 		assert.equal($('p').text().trim(), 'Internal server error');
 	});
+
+	it('generator that throws called in template', async () => {
+		/** @type {Response} */
+		const res = await fixture.fetch('/generator');
+		const reader = res.body.getReader();
+		const decoder = new TextDecoder();
+		const expect = async ({ done, value }) => {
+			const result = await reader.read();
+			assert.equal(result.done, done);
+			if (!done) assert.equal(decoder.decode(result.value), value);
+		}
+		await expect({ done: false, value: "<!DOCTYPE html><h1>Astro</h1> 1Internal server error" });
+		await expect({ done: true });
+	});
 });
diff --git a/packages/integrations/node/test/fixtures/errors/src/pages/generator.astro b/packages/integrations/node/test/fixtures/errors/src/pages/generator.astro
@@ -0,0 +1,11 @@
+---
+function * generator () {
+    yield 1
+    throw Error('ohnoes')
+}
+---
+<h1>Astro</h1>
+{generator()}
+<footer>
+	Footer
+</footer>