Express hangs when a controller method's return value evaluates as false

[rmblstrp]

Expected Behavior

Controller methods that do not return a value or return a NULL value should complete the same as if it returned a value that does not evaluate as false.

Current Behavior

When the controller method does not return a value, or it returns a value that evaluates as false, the method is forced to call response.send() in order to complete the request.

Possible Solution

(async () => {
    // We use a childContainer for each request so we can be
    // sure that the binding is unique for each HTTP request
    let childContainer = this._container.createChild();
    const httpContext = this._getHttpContext(req);
    childContainer.bind<interfaces.HttpContext>(TYPE.HttpContext)
                  .toConstantValue(httpContext);

    // invoke controller's action
    let result = childContainer.getNamed<any>(TYPE.Controller, controllerName)[key](...args);
    Promise.resolve(result)
        .then((value: any) => {
            if (!res.headersSent) {
                if (value === undefined) {
                    res.status(204);
                }
                res.send(value);
            }
        })
        .catch((error: any) => next(error));
})();

Steps to Reproduce (for bugs)

import {Container} from "inversify";
import {controller, httpDelete, interfaces, InversifyExpressServer, requestParam} from "inversify-express-utils";

@controller("/foo")
export class FooController implements interfaces.Controller {
    @httpDelete("/:id")
    private async delete(@requestParam("id") id: string): Promise<void> {
        console.log(`${id} was deleted`);
    }
}

// create server
let server = new InversifyExpressServer(new Container());
server.build().listen(3000);

Context

I converted some of our express Lambda handlers over to be controller based and found that when a method did not return a result or the result evaluated to false then the request would be timed out by the server process. To workaround the issue I am forced to wrap the result of each function to do the same evaluation and call response.send() if the evaluation is false.

Your Environment

• Version used: 5.2.0
• Environment name and version: node.js 8.9.1
• Operating System and version: OSX 10.13.2

[pkeuter]

Hi there!

This PR (inversify/inversify-express-utils#126) breaks piping data like

    stream.data.pipe(_res);
    return;

This is because the headers won't be set yet when returning from the function. There is obviously a workaround by writing the headers before returning, but I liked the previous method of just returning without a value. This also gives the error handler a chance to set the headers (when opening the filestream fails for instance). Is there a way we can reimplement this?

On a sidenote; this was a breaking change, so I would have expected the version number to change to 6.0.0 instead of 5.2.1.

[pkeuter]

There currently (since this PR) seems no way to not let Inversify handle the response. This is obviously a problem. If people really want the possibility to not return anything and still let Inversify handle the response, we need some way to flag that we want to override functionality. This can possibly be done in the decorator? Hopefully we can find a good solution to this, but currently this is broken (since it's not released as a breaking change) and there is no workaround. As such, I would opt for reverting this PR until we find a decent way to handle this situation. @remojansen please advise.