image

A colleague the other day introduced a handy snippet of code enabling async await usage within express route handlers and asked my opinion about it. After thinking quite a bit, I decided to write this post. Here’s the snippet.

const asyncWrap = fn => (req, res, next) =>
	Promise
		.resolve(fn(req, res, next))
		.catch(err => next(err));

const handler = async (req, res) => {
	res.send();
};

app.get('/cats', asyncWrap(handler));

The problem this function solves is that express doesn’t natively handle promises, and so it doesn’t support async / await. Route handlers don’t treat promise rejections as errors which would normally trigger the default express error handling middleware. Express route handlers are wrapped in a try / catch block which catches synchronous unhandled errors and passes them to the default error handling middleware.

app.get('/cats', (req, res) => {
	// would be caught by error handling middleware
	throw new Error('boom');
	// would not be caught by error handling middleware
	return Promise.reject();
});

To leverage async functions, the express library uses callbacks. The error handling callback is a third argument passed to route handlers called a NextFunction. Like most Node.js callbacks When calling this with an error in the first parameter, the middleware chain will end and the default express middleware will be invoked.

app.get('/cats', (req, res, next) => {
	// would go straight to error handling middleware
	return next(new Error('boom'));
});

Very often, developers unknowingly write route handlers using async / await introducing this subtle error handling bug.

const handler = async (req, res) => {
	throw new Error('boom');
};
// will not get caught
app.get('/cats', handler);
// leverages the helper function, errors will be caught
app.get('/cats', asyncWrap(handler));

Another way to visualize this is to implement this inline using an IIFE.

app.get('/cats', (req, res, next) => {
	(async () => {
		const data = await Promise.resolve({});
		return res.send(data);
	})().catch(next);
});

This implementation is fine, however it doesn’t leverage prototypical optimization so a better way to implement this would be to define a private method on a controller class. This way, the main function body doesn’t need to be evaluated on every call.

export class CatCtrl {
	public findAll(req, res, done) {
		this._findAll(req, res).catch(done);
	}
	private async _findAll(req, res) {
		await Promise.resolve();
	}
}

This solution leverages a higher order function to wrap the async function with the express callback handler. It works, but it’s verbose; there’s definitely a cleaner solution.

So, back to the original solution.

// src/CatCtrl.ts
export class CatCtrl {
	public async findAll(req, res) {
		await Promise.resolve();
		res.send({});
	}
}
// src/routes.ts
import { CatCtrl } from' ./CatCtrl';
const asyncWrap = fn => (req, res, next) =>
	Promise
		.resolve(fn(req, res, next))
		.catch(err => next(err));

export const register = (app) => {
	const ctrl = new CatCtrl();
	app.get('/cats', asyncWrap(ctrl.findAll));
}

This implementation is much more terse. We’ve got our Controller class with async route handlers, and we’re catching the errors with the wrapper function. Is this solution perfect? I don’t think so. Here’s why.

While this solution solves the problem in a dry elegant way, the issue I have is that I think this solution effectively adds magic into the codebase, couples the implementation to the infrastructure, and is error prone from a maintainability perspective.

Express route handlers don’t support async functions. Route handler implementations in practice, are almost always declared in a separate file than the route registration within an express application. By enabling what would otherwise be invalid code to be written in one area of the codebase through some distant function call, the code becomes much harder to reason about.

This is analogous to the issues related to using co-mocha. Co-mocha is a package you require at the top of a mocha test file, and it enables async generator functions it('description', function*(){ yield Promise.resolve(); }); to be defined in test suites (popular before async / await was supported). Problems arise, when, a. your entire entire test suite is now coupled to co and mocha and b. more importantly, if you forget to require(‘co-mocha’); at the top of a suite, the tests silently pass even if the underlying implementation is failing.

Back to our async helper problem. The code goes a step beyond inversion of control and is only valid within the context of the infrastructure. If the code was copied into a new route handler, but the handler wasn’t registered with express using the wrapper, the code would be then be incorrect. This is what makes the code coupled to the infrastructure and error prone. As Michael Nygard illustrated in Uncoupling, this type of change introduces a development time coupling because disconnected parts of the application need to be updated in sync in order to work as expected.

I think developers should avoid adding magic in the codebase that mutates or monkey patches the way core language features and libraries act, without it being glaringly obvious or explicit throughout the application. An example of why this would be a problem is that another developer familiar with standard express usage may come along and see an async route handler in production, not knowing of this limitation, copy / paste, and fail to register the route handler with the helper function resulting in buggy code.

// src/DogCtrl.ts
export class DogCtrl {
	public async findAll(req, res) {
		await Promise.resolve();
		res.send({});
	}
}

// src/dogRoutes.ts
import { DogCtrl } from' ./DogCtrl';

export const register = (app) => {
	const dogCtrl = new DogCtrl();
	app.get('/dogs', dogCtrl.findAll); // BUG
}

// src/routes.ts
import { CatCtrl } from' ./CatCtrl';
import { register as dogRoutes } from' ./dogRoutes';

const asyncWrap = fn => (req, res, next) =>
  Promise
    .resolve(fn(req, res, next))
    .catch(err => next(err));

export const register = (app) => {
	const ctrl = new CatCtrl();
	app.get('/cats', asyncWrap(ctrl.findAll));
	dogRoutes(app);
}

So, what’s a more declarative solution? I think placing the wrapper function as close as possible if not in the implementation itself solves this problem because anyone looking at the code given the breadcrumbs to the helper. The way I tackle this here is using ESNext Decorator functions but any of the above implementations will work depending on your application needs.

import { RequestHandler, Request, Response, NextFunction } from 'express';

export const asyncWrap = (routeHandler: RequestHandler) => (req: Request, res: Response, next: NextFunction) =>
	Promise
		.resolve(routeHandler(req, res, next))
		.catch(err => next(err));

export const asyncRequestHandler: MethodDecorator = (target, propertyKey, descriptor) => {
	target[propertyKey] = asyncWrap(target[propertyKey]);
	return descriptor;
};

class CatCtrl {
	@asyncRequestHandler
	public async findAll(req, res, next) {
		res.send('wee');
	}
}

Hopefully this article has highlighted some of the caveats of using ES6 Promises with Express. It’s worth noting that in Express 5 (not yet released) will be support for promises (see https://github.com/expressjs/express/issues/2259) but in the meantime, let’s keep the magic to a minimum.

Thank you for reading! To be the first notified when I publish a new article, sign up for my mailing list!

Ben Lugavere is a Lead Engineer and Architect at Boxed where he develops primarily using JavaScript and TypeScript. Ben writes about all things JavaScript, system design and architecture and can be found on twitter at @benlugavere.

Follow me on medium for more articles or on GitHub to see my open source contributions!