Errors thrown by express middleware on JSONController routes are not caught properly

[prodigga]

Hi there

I noticed that if I throw an error inside of a route, it is caught and the client receives a JSON body response. But if I throw an error inside of some middleware, UseBefore the route, then the error is returned as the body of the response.

For example, this is the body of the response from a custom middleware (AuthrizePlayer) that throws an error:

Error
at AuthorizePlayer.use (C:\Users\Tim\Desktop\Heroku\cargame2020\dist\Middleware\AuthorizePlayer.js:6:15)
at C:\Users\Tim\Desktop\Heroku\cargame2020\node_modules\routing-controllers\driver\express\ExpressDriver.js:381:86
at Layer.handle [as handle_request]
(C:\Users\Tim\Desktop\Heroku\cargame2020\node_modules\express\lib\router\layer.js:95:5)
at next (C:\Users\Tim\Desktop\Heroku\cargame2020\node_modules\express\lib\router\route.js:137:13)
at Route.dispatch (C:\Users\Tim\Desktop\Heroku\cargame2020\node_modules\express\lib\router\route.js:112:3)
at Layer.handle [as handle_request]
(C:\Users\Tim\Desktop\Heroku\cargame2020\node_modules\express\lib\router\layer.js:95:5)
at C:\Users\Tim\Desktop\Heroku\cargame2020\node_modules\express\lib\router\index.js:281:22
at Function.process_params (C:\Users\Tim\Desktop\Heroku\cargame2020\node_modules\express\lib\router\index.js:335:12)
at next (C:\Users\Tim\Desktop\Heroku\cargame2020\node_modules\express\lib\router\index.js:275:10)
at expressInit (C:\Users\Tim\Desktop\Heroku\cargame2020\node_modules\express\lib\middleware\init.js:40:5)
at Layer.handle [as handle_request]
(C:\Users\Tim\Desktop\Heroku\cargame2020\node_modules\express\lib\router\layer.js:95:5)
at trim_prefix (C:\Users\Tim\Desktop\Heroku\cargame2020\node_modules\express\lib\router\index.js:317:13)
at C:\Users\Tim\Desktop\Heroku\cargame2020\node_modules\express\lib\router\index.js:284:7
at Function.process_params (C:\Users\Tim\Desktop\Heroku\cargame2020\node_modules\express\lib\router\index.js:335:12)
at next (C:\Users\Tim\Desktop\Heroku\cargame2020\node_modules\express\lib\router\index.js:275:10)
at query (C:\Users\Tim\Desktop\Heroku\cargame2020\node_modules\express\lib\middleware\query.js:45:5)

Compared to the same error (new Error()) being throw inside of the route itself:

{
    "name": "Error",
    "stack": "Error\n    at PlayerController.GetMyName (C:\\Users\\Tim\\Desktop\\Heroku\\cargame2020\\dist\\Controllers\\PlayerController.js:40:19)\n    at ActionMetadata.callMethod (C:\\Users\\Tim\\Desktop\\Heroku\\cargame2020\\node_modules\\routing-controllers\\metadata\\ActionMetadata.js:108:48)\n    at C:\\Users\\Tim\\Desktop\\Heroku\\cargame2020\\node_modules\\routing-controllers\\RoutingControllers.js:99:140"
}

[prodigga]

Is this intended? And if so, how can I catch the middleware errors and handle them the same way. Or what is the intended way of handling these errors? How can my middleware return an error that is parsed and returned to the user in a way that conforms to the rest of my API?

[prodigga]

Also worth noting that this (incorrect, non JSON) middleware error handler only triggers if my middleware implements ExpressMiddlewareInterface. If I do not implement ExpressMiddlewareInterface and write a middleware as a function, no error handling is done, the route never returns a response to the user.

[prodigga]

I had a look into this. I only only just getting started with Typescript, so I don't feel comfortable enough to contribute. But what I found was that when a Middleware throws an error it does not receive any ActionMetadata. The ActionMetadatais what tells the default error handler whether the route is JSON or not.

Here is the error catching for middlewares, and undefined is passed into the error handler for ActionMetadata.

routing-controllers/src/driver/express/ExpressDriver.ts

Lines 67 to 80 in 5326428

	middlewareWrapper = (request: any, response: any, next: (err: any) => any) => {
	try {
	const useResult = (middleware.instance as ExpressMiddlewareInterface).use(request, response, next);
	if (isPromiseLike(useResult)) {
	useResult.catch((error: any) => {
	this.handleError(error, undefined, {request, response, next});
	return error;
	});
	}
	} catch (error) {
	this.handleError(error, undefined, {request, response, next});
	}
	};

Here is the error handling code, where the isJsonTypes check can only be performed if we have a valid ActionMetadata.

routing-controllers/src/driver/express/ExpressDriver.ts

Lines 357 to 384 in 5326428

	handleError(error: any, action: ActionMetadata | undefined, options: Action): any {
	if (this.isDefaultErrorHandlingEnabled) {
	const response: any = options.response;
	// set http code
	// note that we can't use error instanceof HttpError properly anymore because of new typescript emit process
	if (error.httpCode) {
	response.status(error.httpCode);
	} else {
	response.status(500);
	}
	// apply http headers
	if (action) {
	Object.keys(action.headers).forEach(name => {
	response.header(name, action.headers[name]);
	});
	}
	// send error content
	if (action && action.isJsonTyped) {
	response.json(this.processJsonError(error));
	} else {
	response.send(this.processTextError(error)); // todo: no need to do it because express by default does it
	}
	}
	options.next(error);
	}

In my personal use case, every route is JSON (Rest API). I think I can skirt around this issue with a custom error handler that is a copy of the default error handler and simply assumes every action returns a JSON result.

[Kalhama]

I'm also finding this troublesome. I think middlewares and controllers should work in a similar manner.

I also noticed that returning routing-controllers custom errors (ie. throw UnauthorizedError()) yields HTTP 500 when it should return HTTP 401.

I modiefied some code that already exists in the project repo in order to have a workaround until valid stable fix is provided. Example of this workaround attached below.

import { Response } from 'express'
import { HttpError } from 'routing-controllers'

const processJsonError = (error: any): any => {
    if (typeof error.toJSON === 'function') return error.toJSON()

    let processedError: any = {}
    if (error instanceof Error) {
        const name = error?.name !== 'Error' ? error.name : error.constructor.name
        processedError.name = name

        if (error.message) processedError.message = error.message
        if (error.stack && process.env.NODE_ENV !== 'production') processedError.stack = error.stack

        Object.keys(error)
            .filter(
                (key) =>
                    key !== 'stack' &&
                    key !== 'name' &&
                    key !== 'message' &&
                    (!(error instanceof HttpError) || key !== 'httpCode')
            )
            .forEach((key) => (processedError[key] = (error as any)[key]))

        if (this.errorOverridingMap)
            Object.keys(this.errorOverridingMap)
                .filter((key) => name === key)
                .forEach((key) => (processedError = this.merge(processedError, this.errorOverridingMap[key])))

        return Object.keys(processedError).length > 0 ? processedError : undefined
    }

    return error
}

export const middlewareErrorHandler = (error, response: Response): void => {
    response.status(error.httpCode || 500)
    response.json(processJsonError(error))
}

USAGE EXAMPLE

import { ExpressMiddlewareInterface } from 'routing-controllers'
import { Request, Response } from 'express'
import { auth} from '../auth'
import { middlewareErrorHandler } from './common/middlewareErrorHandler'

export class UserMiddleware implements ExpressMiddlewareInterface {
    async use(request: Request, response: Response, next: (err?) => void): Promise<void> {
        auth()
            .then(() => next())
            .catch((err) => middlewareErrorHandler(err, response))
    }
}

[ThePooN]

Also happens to Koa middlewares.

This issue is a deal-breaker for me :(

[attilaorosz]

@prodigga I looked into this and this is basically a limitation at the moment. You get a JSON formatted error from the route because probably it is decorated with JsonController. If you throw an error in the middleware, it has no idea about your reponse type so it falls back to text, like the Controller decorator.

If you would like to format your errors the same in each scenario, you have to use a custom error handler.

[attilaorosz]

Closing as stale.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.