Uncaught error in async endpoint crashes express

[mvarendorff]

Bug Report

Any uncaught errors in an async handler function of an endpoint will crash the underlying express instance (however not the process as it seems). I understand that this is something that is flawed in express and should be fixed upstream however the Payload docs themselves use an async endpoint in the docs without any mention of this issue so I feel like it should be handled until express 5 becomes stable.

Workaround in the form of a plugin is included below.

Steps to Reproduce

1. Add these two innocently looking endpoints to the Payload config:

endpoints: [
  {
    path: "/poof",
    method: "get",
    handler: (req, res, next) => {
      throw new Error("Poof")
    },
  },
  {
    path: "/crash",
    method: "get",
    handler: async (req, res, next) => {
      throw new Error("Crash")
    },
  },
],

2. Open http://localhost:3000/api/poof -> The result is a 500 error saying Poof
3. Open http://localhost:3000/api/crash -> There is no response and any further requests don't produce a response anymore either.

Other Details

After some digging, it turns out that Express 4 does not handle errors in async route handlers which will be fixed in Express 5 however that's been in beta for a year now (related issue with maintainer updates: expressjs/express#4920).

My two ideas for resolution of this would be:

1. Use express 5 beta (although I understand that this is probably not really an option for a project of this scale)
2. When mounting the handlers, wrap them in an async handler that takes care of catching the error and handling it by passing it to next with something like this:

const safeAsyncHandler = (handler: PayloadHandler): PayloadHandler => async (req, res, next) => {
  try {
    await handler(req, res, next);
  } catch (e) {
    next(e);
  }
};

And then using that function in here (maybe somewhere else, I haven't looked far and wide but this seemed like a reasonable guess):

payload/src/express/mountEndpoints.ts

Lines 4 to 12 in 4d578f1

	function mountEndpoints(express: Express, router: Router, endpoints: Endpoint[]): void {
	endpoints.forEach((endpoint) => {
	if (!endpoint.root) {
	router[endpoint.method](endpoint.path, endpoint.handler);
	} else {
	express[endpoint.method](endpoint.path, endpoint.handler);
	}
	});
	}

Payload version: 1.6.17

Workaround (Payload plugin)

Using the function from above, it's possible to write a plugin as workaround that handles async errors gracefully:

import {type Config, Endpoint, PayloadHandler} from "payload/config";

const safeAsyncHandler = (handler: PayloadHandler): PayloadHandler => async (req, res, next) => {
  try {
    await handler(req, res, next);
  } catch (e) {
    next(e);
  }
};

const patchEndpoint = (endpoint: Endpoint): Endpoint => {
  const handler = Array.isArray(endpoint.handler)
    ? endpoint.handler.map(safeAsyncHandler)
    : safeAsyncHandler(endpoint.handler);

  return {...endpoint, handler};
}

export const safeAsyncEndpoints = (config: Config) => {
  config.endpoints = config.endpoints?.map(patchEndpoint) ?? [];
  for (const c of (config.collections ?? [])) {
    c.endpoints = c.endpoints?.map(patchEndpoint) ?? [];
  }

  for (const g of (config.globals ?? [])) {
    g.endpoints = g.endpoints?.map(patchEndpoint) ?? [];
  }

  return config;
}

// Usage: plugins: [safeAsyncEndpoints] in payload.config.ts

[jmikrut]

Hey there! This is a great topic and the amount of information you've provided here is awesome.

I think personally the way to go would be your Option 2:

When mounting the handlers, wrap them in an async handler that takes care of catching the error and handling it

But I wonder, because having a try / catch in your own handler is typically always going to be required, is this duplicative? I think best practice would be to handle your own errors (thus controlling logs, resulting REST responses, etc.) It's an interesting thought.

Would certainly clean up custom handlers if you don't have to wrap every single one in a top-level try / catch.

I would love to get more feedback on this idea. I'll take it to my team and share it with Discord.

I will also convert this to a Discussion and tag it as a Feature Request.

Thanks for all of the notes. The plugin is great too!

[StLyn4]

This can be solved with just two lines:

1. yarn add express-async-errors.
2. import 'express-async-errors'; in server.ts.

For example, my server.ts starts like this (this also applies to JS):

import express from 'express';
import payload from 'payload';

import 'dotenv/config';
import 'express-async-errors';