From 018014f0bed9fddafea03a49bb13089461cffe87 Mon Sep 17 00:00:00 2001 From: lilnasy <69170106+lilnasy@users.noreply.github.com> Date: Wed, 27 Dec 2023 18:28:35 +0000 Subject: [PATCH 1/3] fix(node): prevent crash on stream error --- .../integrations/node/src/nodeMiddleware.ts | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/packages/integrations/node/src/nodeMiddleware.ts b/packages/integrations/node/src/nodeMiddleware.ts index 0b9381f1d0a2..b78d57e62661 100644 --- a/packages/integrations/node/src/nodeMiddleware.ts +++ b/packages/integrations/node/src/nodeMiddleware.ts @@ -2,6 +2,7 @@ import type { NodeApp } from 'astro/app/node'; import type { ServerResponse } from 'node:http'; import { createOutgoingHttpHeaders } from './createOutgoingHttpHeaders.js'; import type { ErrorHandlerParams, Options, RequestHandlerParams } from './types.js'; +import type { AstroIntegrationLogger } from 'astro'; // Disable no-unused-vars to avoid breaking signature change export default function (app: NodeApp, mode: Options['mode']) { @@ -29,12 +30,14 @@ export default function (app: NodeApp, mode: Options['mode']) { } } + const logger = app.getAdapterLogger(); + try { const routeData = app.match(req); if (routeData) { try { const response = await app.render(req, { routeData, locals }); - await writeWebResponse(app, res, response); + await writeWebResponse(app, res, response, logger); } catch (err: unknown) { if (next) { next(err); @@ -46,10 +49,9 @@ export default function (app: NodeApp, mode: Options['mode']) { return next(); } else { const response = await app.render(req); - await writeWebResponse(app, res, response); + await writeWebResponse(app, res, response, logger); } } catch (err: unknown) { - const logger = app.getAdapterLogger(); logger.error(`Could not render ${req.url}`); console.error(err); if (!res.headersSent) { @@ -60,26 +62,32 @@ export default function (app: NodeApp, mode: Options['mode']) { }; } -async function writeWebResponse(app: NodeApp, res: ServerResponse, webResponse: Response) { - const { status, headers } = webResponse; +async function writeWebResponse(app: NodeApp, res: ServerResponse, webResponse: Response, logger: AstroIntegrationLogger) { + const { status, headers, body } = webResponse; if (app.setCookieHeaders) { const setCookieHeaders: Array = Array.from(app.setCookieHeaders(webResponse)); if (setCookieHeaders.length) { for (const setCookieHeader of setCookieHeaders) { - webResponse.headers.append('set-cookie', setCookieHeader); + headers.append('set-cookie', setCookieHeader); } } } const nodeHeaders = createOutgoingHttpHeaders(headers); res.writeHead(status, nodeHeaders); - if (webResponse.body) { + if (body) { try { - const reader = webResponse.body.getReader(); + const reader = body.getReader(); res.on('close', () => { - reader.cancel(); + // Cancelling the reader may reject not just because of + // an error in the ReadableStream's cancel callback, but + // also because of an error anywhere in the stream. + reader.cancel().catch(err => { + logger.error(`There was an uncaught error in the middle of the stream while rendering ${res.req.url}.`); + console.error(err); + }); }); let result = await reader.read(); while (!result.done) { @@ -87,7 +95,6 @@ async function writeWebResponse(app: NodeApp, res: ServerResponse, webResponse: result = await reader.read(); } } catch (err: any) { - console.error(err?.stack || err?.message || String(err)); res.write('Internal server error'); } } From b94d1af62450652fe6eb4b7fbac4503e5bf1b0a3 Mon Sep 17 00:00:00 2001 From: lilnasy <69170106+lilnasy@users.noreply.github.com> Date: Wed, 27 Dec 2023 18:34:32 +0000 Subject: [PATCH 2/3] add changeset --- .changeset/olive-sloths-brush.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/olive-sloths-brush.md diff --git a/.changeset/olive-sloths-brush.md b/.changeset/olive-sloths-brush.md new file mode 100644 index 000000000000..574af00902b0 --- /dev/null +++ b/.changeset/olive-sloths-brush.md @@ -0,0 +1,5 @@ +--- +'@astrojs/node': patch +--- + +Fixes a bug where an error while serving response stopped the server. From 9d64ede18bf579f5ef756425ca3ae6d994ff09ae Mon Sep 17 00:00:00 2001 From: Arsh <69170106+lilnasy@users.noreply.github.com> Date: Fri, 29 Dec 2023 20:32:48 +0530 Subject: [PATCH 3/3] Apply suggestions from code review --- packages/integrations/node/src/nodeMiddleware.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/integrations/node/src/nodeMiddleware.ts b/packages/integrations/node/src/nodeMiddleware.ts index b78d57e62661..f1fe50d7667e 100644 --- a/packages/integrations/node/src/nodeMiddleware.ts +++ b/packages/integrations/node/src/nodeMiddleware.ts @@ -94,7 +94,8 @@ async function writeWebResponse(app: NodeApp, res: ServerResponse, webResponse: res.write(result.value); result = await reader.read(); } - } catch (err: any) { + // the error will be logged by the "on end" callback above + } catch { res.write('Internal server error'); } }