From 2f6d1faa6f2d6de2d4ccd2a48adf5adadc82e593 Mon Sep 17 00:00:00 2001 From: Arsh <69170106+lilnasy@users.noreply.github.com> Date: Thu, 1 Feb 2024 07:02:40 +0000 Subject: [PATCH] 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 --- .changeset/clever-roses-sleep.md | 5 +++++ packages/astro/src/core/app/node.ts | 4 ++-- .../integrations/node/test/errors.test.js | 19 +++++++++++++++++-- .../fixtures/errors/src/pages/generator.astro | 11 +++++++++++ 4 files changed, 35 insertions(+), 4 deletions(-) create mode 100644 .changeset/clever-roses-sleep.md create mode 100644 packages/integrations/node/test/fixtures/errors/src/pages/generator.astro diff --git a/.changeset/clever-roses-sleep.md b/.changeset/clever-roses-sleep.md new file mode 100644 index 000000000000..fd159eb3ba34 --- /dev/null +++ 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 index ddfa70997c4a..db61157e319e 100644 --- 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 index 983187475799..23deba02cce4 100644 --- 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: "

Astro

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 new file mode 100644 index 000000000000..65b8ae62c15d --- /dev/null +++ b/packages/integrations/node/test/fixtures/errors/src/pages/generator.astro @@ -0,0 +1,11 @@ +--- +function * generator () { + yield 1 + throw Error('ohnoes') +} +--- +

Astro

+{generator()} + \ No newline at end of file