Skip to content

Commit

Permalink
fix(node): handle offshoot promise rejections (#10454)
Browse files Browse the repository at this point in the history
* fix(node): handle offshoot promise rejections

* add test

* add changeset

* Update packages/integrations/node/test/errors.test.js
  • Loading branch information
lilnasy authored Mar 18, 2024
1 parent 4c1edd0 commit 83f9105
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 5 deletions.
5 changes: 5 additions & 0 deletions .changeset/sharp-flowers-ring.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@astrojs/node": patch
---

Prevents crashes caused by rejections of offshoot promises.
19 changes: 16 additions & 3 deletions packages/integrations/node/src/serve-app.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { AsyncLocalStorage } from 'node:async_hooks';
import { NodeApp } from 'astro/app/node';
import type { RequestHandler } from './types.js';

Expand All @@ -7,8 +8,20 @@ import type { RequestHandler } from './types.js';
* Intended to be used in both standalone and middleware mode.
*/
export function createAppHandler(app: NodeApp): RequestHandler {
/**
* Keep track of the current request path using AsyncLocalStorage.
* Used to log unhandled rejections with a helpful message.
*/
const als = new AsyncLocalStorage<string>();
const logger = app.getAdapterLogger();
process.on('unhandledRejection', reason => {
const requestUrl = als.getStore();
logger.error(`Unhandled rejection while rendering ${requestUrl}`);
console.error(reason);
});

return async (req, res, next, locals) => {
let request;
let request: Request;
try {
request = NodeApp.createRequest(req);
} catch (err) {
Expand All @@ -19,11 +32,11 @@ export function createAppHandler(app: NodeApp): RequestHandler {

const routeData = app.match(request);
if (routeData) {
const response = await app.render(request, {
const response = await als.run(request.url, () => app.render(request, {
addCookieHeader: true,
locals,
routeData,
});
}));
await NodeApp.writeResponse(response, res);
} else if (next) {
return next();
Expand Down
30 changes: 28 additions & 2 deletions packages/integrations/node/test/errors.test.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
import { spawn } from 'node:child_process';
import { Worker } from 'node:worker_threads';
import assert from 'node:assert/strict';
import { after, before, describe, it } from 'node:test';
import * as cheerio from 'cheerio';
import nodejs from '../dist/index.js';
import { loadFixture } from './test-utils.js';
import { fileURLToPath } from 'node:url';

describe('Errors', () => {
/** @type {import('./test-utils.js').Fixture} */
let fixture;

before(async () => {
fixture = await loadFixture({
root: './fixtures/errors/',
Expand All @@ -17,10 +22,31 @@ describe('Errors', () => {
let devPreview;

before(async () => {
devPreview = await fixture.preview();
// The two tests that need the server to run are skipped
// devPreview = await fixture.preview();
});
after(async () => {
await devPreview.stop();
await devPreview?.stop();
});

it('stays alive after offshoot promise rejections', async () => {
// this test needs to happen in a worker because node test runner adds a listener for unhandled rejections in the main thread
const worker = new Worker('./test/fixtures/errors/dist/server/entry.mjs', {
type: 'module',
env: { ASTRO_NODE_LOGGING: 'enabled' }
});

await new Promise((resolve, reject) => {
worker.stdout.on('data', data => {
setTimeout(() => reject("Server took too long to start"), 1000);
if (data.toString().includes('Server listening on http://localhost:4321')) resolve();
});
});

await fetch("http://localhost:4321/offshoot-promise-rejection");

// if there was a crash, it becomes an error here
await worker.terminate();
});

it(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{new Promise(async _ => (await {}, Astro.props.undefined.alsoAPropertyOfUndefined))}
{Astro.props.undefined.propertyOfUndefined}

0 comments on commit 83f9105

Please sign in to comment.