Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle unhandledrejections in the dev server #9424

Merged
merged 6 commits into from
Dec 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/curvy-lobsters-crash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Prevents dev server from crashing on unhandled rejections, and adds a helpful error message
9 changes: 9 additions & 0 deletions packages/astro/src/core/errors/errors-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1292,3 +1292,12 @@ export const CantRenderPage = {

// Generic catch-all - Only use this in extreme cases, like if there was a cosmic ray bit flip
export const UnknownError = { name: 'UnknownError', title: 'Unknown Error.' } satisfies ErrorData;

export const UnhandledRejection = {
name: 'UnhandledRejection',
title: 'Unhandled rejection',
message: (stack: string) => {
return `Astro detected an unhandled rejection. Here's the stack trace:\n${stack}`;
},
hint: 'Make sure your promises all have an `await` or a `.catch()` handler.'
}
33 changes: 33 additions & 0 deletions packages/astro/src/vite-plugin-astro-server/error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import type { ModuleLoader } from '../core/module-loader/index.js'
import type { AstroConfig } from '../@types/astro.js';
import type DevPipeline from './devPipeline.js';

import { collectErrorMetadata } from '../core/errors/dev/index.js';
import { createSafeError } from '../core/errors/index.js';
import { formatErrorMessage } from '../core/messages.js';
import { eventError, telemetry } from '../events/index.js';

export function recordServerError(loader: ModuleLoader, config: AstroConfig, pipeline: DevPipeline, _err: unknown) {
const err = createSafeError(_err);

// This could be a runtime error from Vite's SSR module, so try to fix it here
try {
loader.fixStacktrace(err);
} catch {}

// This is our last line of defense regarding errors where we still might have some information about the request
// Our error should already be complete, but let's try to add a bit more through some guesswork
const errorWithMetadata = collectErrorMetadata(err, config.root);

telemetry.record(eventError({ cmd: 'dev', err: errorWithMetadata, isFatal: false }));

pipeline.logger.error(
null,
formatErrorMessage(errorWithMetadata, pipeline.logger.level() === 'debug')
);

return {
error: err,
errorWithMetadata
};
}
39 changes: 32 additions & 7 deletions packages/astro/src/vite-plugin-astro-server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ import { baseMiddleware } from './base.js';
import { createController } from './controller.js';
import DevPipeline from './devPipeline.js';
import { handleRequest } from './request.js';
import { AstroError, AstroErrorData } from '../core/errors/index.js';
import { getViteErrorPayload } from '../core/errors/dev/index.js';
import { AsyncLocalStorage } from 'node:async_hooks';
import { IncomingMessage } from 'node:http';
import { setRouteError } from './server-state.js';
import { recordServerError } from './error.js';

export interface AstroPluginOptions {
settings: AstroSettings;
Expand All @@ -30,6 +36,7 @@ export default function createVitePluginAstroServer({
const pipeline = new DevPipeline({ logger, manifest, settings, loader });
let manifestData: ManifestData = createRouteManifest({ settings, fsMod }, logger);
const controller = createController({ loader });
const localStorage = new AsyncLocalStorage();

/** rebuild the route cache + manifest, as needed. */
function rebuildManifest(needsManifestRebuild: boolean) {
Expand All @@ -43,6 +50,22 @@ export default function createVitePluginAstroServer({
viteServer.watcher.on('unlink', rebuildManifest.bind(null, true));
viteServer.watcher.on('change', rebuildManifest.bind(null, false));

function handleUnhandledRejection(rejection: any) {
const error = new AstroError({
...AstroErrorData.UnhandledRejection,
message: AstroErrorData.UnhandledRejection.message(rejection?.stack || rejection)
});
const store = localStorage.getStore();
if(store instanceof IncomingMessage) {
const request = store;
setRouteError(controller.state, request.url!, error);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This marks the route as being in an error state.

}
const { errorWithMetadata } = recordServerError(loader, settings.config, pipeline, error);
setTimeout(async () => loader.webSocketSend(await getViteErrorPayload(errorWithMetadata)), 200)
}

process.on('unhandledRejection', handleUnhandledRejection);

return () => {
// Push this middleware to the front of the stack so that it can intercept responses.
// fix(#6067): always inject this to ensure zombie base handling is killed after restarts
Expand All @@ -57,13 +80,15 @@ export default function createVitePluginAstroServer({
response.end();
return;
}
handleRequest({
pipeline,
manifestData,
controller,
incomingRequest: request,
incomingResponse: response,
manifest,
localStorage.run(request, () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing in the request which acts as the context for the local storage. When an unhandled rejection occurs, we can use this to know which request it was associated with.

Copy link
Member

@ematipico ematipico Dec 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First time I see AsyncLocalStorage in action!

handleRequest({
pipeline,
manifestData,
controller,
incomingRequest: request,
incomingResponse: response,
manifest,
});
});
});
};
Expand Down
22 changes: 3 additions & 19 deletions packages/astro/src/vite-plugin-astro-server/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { runWithErrorHandling } from './controller.js';
import type DevPipeline from './devPipeline.js';
import { handle500Response } from './response.js';
import { handleRoute, matchRoute } from './route.js';
import { recordServerError } from './error.js';

type HandleRequest = {
pipeline: DevPipeline;
Expand Down Expand Up @@ -89,26 +90,9 @@ export async function handleRequest({
});
},
onError(_err) {
const err = createSafeError(_err);

// This could be a runtime error from Vite's SSR module, so try to fix it here
try {
moduleLoader.fixStacktrace(err);
} catch {}

// This is our last line of defense regarding errors where we still might have some information about the request
// Our error should already be complete, but let's try to add a bit more through some guesswork
const errorWithMetadata = collectErrorMetadata(err, config.root);

telemetry.record(eventError({ cmd: 'dev', err: errorWithMetadata, isFatal: false }));

pipeline.logger.error(
null,
formatErrorMessage(errorWithMetadata, pipeline.logger.level() === 'debug')
);
const { error, errorWithMetadata } = recordServerError(moduleLoader, config, pipeline, _err);
handle500Response(moduleLoader, incomingResponse, errorWithMetadata);

return err;
return error;
},
});
}