ErrorHandlerMiddleware is ignore with 6.1.*

[VincentClair]

Sorting

• I'm submitting a ...

  • bug report
  • feature request
  • support request

• I confirm that I

  • used the search to make sure that a similar issue hasn't already been submit

Expected Behavior

When an error is thrown, custom ErrorHandlerMiddleware catches the error and returns a response with according errorCode (400, 403, 429, etc.). Works as expected with 6.0.*

Current Behavior

When an error is thrown, custom ErrorHandlerMiddleware is ignore and app hangs up, nothing happens. Breaks with upgrade to 6.1.4 and 6.1.5, and routes regenerated.

Context (Environment)

Version of the library: 6.1.*
Version of NodeJS: 20.*

• Confirm you were using yarn not npm: [ ]

Breaking change?

It seems that is related to the new way of generated routes that handles requests.
I have passed many hours to debug, but does not find something really more helpfull. Sorry !

[github-actions]

Hello there VincentClair 👋

Thank you for opening your very first issue in this project.

We will try to get back to you as soon as we can.👀

[VincentClair]

Code example :

import * as express from 'express';
import * as http from 'http';
import { APIError } from '../Error/APIError';

const app = express();
const server = http.createServer(app);

RegisterRoutes(app);

app.use((
        err: APIError,
        req: express.Request,
        res: express.Response,
        next: express.NextFunction,
    ) => {
        try {
            if (res.headersSent) {
                next(err);
                return;
            }

            res.status(err.code)
                .json({ success: false });
        } catch (e) {
            console.debug(e);
            res.status(500);
            next(e);
        }
    });

server.listen(8080, () => {
    logger.info(`Server listening on port: ${port}`);
});

import { Get, Route, Security } from 'tsoa';
import { APIError, ForbiddenAPIError, provideSingleton } from '../../src';

@Route('test')
@provideSingleton(TestController)
export class TestController {
    @Get('/success')
    public success(): Promise<{ success: true }> {
        // works as expected
        return Promise.resolve({ success: true });
    }

    @Get('/conflict')
    public conflict(): Promise<void> {
        // fails / hangs
        throw new APIError('test-conflict', null, 409);
    }
}

[VincentClair]

Find the soluton : put buildPromise inside try/catch

tsoa/packages/runtime/src/routeGeneration/templates/express/expressTemplateService.ts

Line 41 in efdb468

const promise = this.buildPromise(methodName, controller, validatedArgs);

tsoa/packages/runtime/src/routeGeneration/templates/koa/koaTemplateService.ts

Line 43 in efdb468

const promise = this.buildPromise(methodName, controller, validatedArgs);

tsoa/packages/runtime/src/routeGeneration/templates/hapi/hapiTemplateService.ts

Line 47 in efdb468

const promise = this.buildPromise(methodName, controller, validatedArgs);

    async apiHandler(params) {
        const { methodName, controller, response, validatedArgs, successStatus, next } = params;
        try {
            const promise = this.buildPromise(methodName, controller, validatedArgs);
            const data = await Promise.resolve(promise);
            let statusCode = successStatus;
            let headers;
            if (this.isController(controller)) {
                headers = controller.getHeaders();
                statusCode = controller.getStatus() || statusCode;
            }
            this.returnHandler({ response, headers, statusCode, data });
        }
        catch (error) {
            return next(error);
        }
    }

[jackey8616]

@VincentClair thank you for locate the missing part, could you submit a PR including test cases let tsoa prevent such issue in the future?

[VincentClair]

@jackey8616 have you an advice how to test this case ?
I suppose it's more an integration test to put in *-server.spec.ts ?

[jackey8616]

@jackey8616 have you an advice how to test this case ?

I suppose it's more an integration test to put in *-server.spec.ts ?

Yes, you should add one test case in express, koa, hapi integration test.

You can take fixtures/getController.ts as example, duplicate the method with throwing an error and add handle case in .spec.ts of 3 of that framework.

[VincentClair]

Sorry, but i haven't find a way to add a proper test and I have no more time to pass on it.

It seems the solution has to be completed: when we use inversify (and probably all other solutions ?) we have to await for apiHandler to execute, so errors could be catched and pass to next(err). Otherwise, errors could pass through tr/catch due to async code.

tsoa/packages/cli/src/routeGeneration/templates/express.hbs

Line 106 in efdb468

templateService.apiHandler({

{{#if ../../iocModule}}async {{/if}}templateService.apiHandler({

[jackey8616]

Sorry, but i haven't find a way to add a proper test and I have no more time to pass on it.

It seems the solution has to be completed: when we use inversify (and probably all other solutions ?) we have to await for apiHandler to execute, so errors could be catched and pass to next(err). Otherwise, errors could pass through tr/catch due to async code.

tsoa/packages/cli/src/routeGeneration/templates/express.hbs

Line 106 in efdb468

templateService.apiHandler({

{{#if ../../iocModule}}async {{/if}}templateService.apiHandler({

Never mind, I will take rest of things, thanks for your informations.
I will submit a PR to fix this ASAP.

[jackey8616]

@VincentClair
can you provides more context, everything seems works fine with error handler.
test cases is covered with throwing error inside controller, and it can be correctly catched from outside global error handler.

[VincentClair]

Finally found the problem: if the throwing method is not async, error is neither caught by apiHandler nor generated template.

I have made a PR #1602

[VincentClair]

@WoH When did you plan to publish the fix please ?