Inversifyjs: Express crashes with ERR_HTTP_HEADERS_SENT with async controller function

[johnheroy]

TL;DR

This looks almost like #241 (headers sent crash because async API not supported), so this could be a possible regression from ~2 years ago. I want to be able to use the declarative syntax and DI of Inversify and the express utils, as well as writing my controller methods with RxJS in an async classic Express style (sample below).

Expected Behavior

Should be able to not return anything and async call a method on express.Response instance in controller methods.

Current Behavior

App crashes with Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client

Possible Solution

Not sure yet...haven't dug much into the express server code but didn't see anything obvious in the decorators.ts in the express utils repo.

Steps to Reproduce (for bugs)

Sample code

import { controller, httpGet, response, request, queryParam } from 'inversify-express-utils'
import { Request, Response } from 'express'
import { Hello } from '../entity/Hello'
import { getRepository, Repository } from 'typeorm'
import { from } from 'rxjs'
import { ignoreElements } from 'rxjs/operators'

@controller('/hello')
export class HelloController {

  private repository: Repository<Hello> = getRepository(Hello)

  @httpGet('/')
  public home(
      @request() req: Request, 
      @response() res: Response,
      @queryParam('name') name: string): Promise<void> {

    name = name || 'World'
    let user = this.repository.create({ name: name })

    let observable = from(this.repository.insert(user))
    observable.subscribe((hello) => {
      res.send(`Hello ${name}`)
    }, (error) => {
      res.status(500).end()
    })

    // Crashes without this line
    return observable.pipe(ignoreElements()).toPromise()
  }

  @httpGet('/all')
  public all(req: Request, res: Response): Promise<void> {
    let observable = from(this.repository.find())
    observable.subscribe((hellos: Hello[]) => { 
      res.json(hellos)
    }, (error) => {
      res.status(500).end()
    })

    // Crashes without this line
    return observable.pipe(ignoreElements()).toPromise() // Hacky workaround
  }
}

Context

I want to use RxJS and the power of RX operators in express projects and see how far I can push it (I work on RxJava + Kotlin at my day job, meaning I get spoiled with awesome projects like Dagger and RxJava for DI and reactive programming), and I want to be able to use RxJS in a meaningful way in a Node.js environment. I just recently came across Inversify, and it seems like a really cool project especially with the Express utils so we can write super declarative resource/controller classes for RESTful APIs.

I found a hacky workaround, which is to just return a promise from the Observable which wrapped the Promise I got from a TypeORM repository (so I am really fighting both Inversify and TypeORM APIs in both directions...).

Your Environment

• Version used: inversify 5.0.1 and inversify-express-utils 6.2.0
• Environment name and version (e.g. Chrome 39, node.js 5.4): node v10.13.0
• Operating System and version (desktop or mobile): Windows 10 (WSL with Ubuntu 16.04)
• Link to your project:

Stack trace

Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client
[1]     at ServerResponse.setHeader (_http_outgoing.js:470:11)
[1]     at ServerResponse.header (~/Workspace/node-starter/node_modules/inversify-express-utils/node_modules/express/lib/response.js:767:10)
[1]     at ServerResponse.contentType (~/Workspace/node-starter/

[dcavanagh]

@johnheroy, from a quick look at your example I see something that might be the problem. return observable.pipe(ignoreElements()).toPromise() triggers the response to be sent with the result of that as the body. You might be able to return await observable.pipe(ignoreElements()).toPromise() or something like that. another option would be to return from inside your subscription handler

[johnheroy]

@dcavanagh Sorry should have been more clear in the original issue post. The code snippet does not crash and sets a correct response (hence "hacky workaround" comment) but I'd like to be able to avoid having those lines observable.pipe(ignoreElements()).toPromise() altogether.

[johnheroy]

I'm getting a 204 response and after reading through the code for InversifyExpressServer, I'm fairly certain the issue is this line:

https://github.com/inversify/inversify-express-utils/blob/master/src/server.ts#L266

// invoke controller's action
const value = await childContainer.getNamed<any>(TYPE.Controller, controllerName)[key](...args);

if (value instanceof HttpResponseMessage) {
    ...
} else if (!res.headersSent) {
    if (value === undefined) {
        res.status(204);
    }
    res.send(value);
}

So it looks like if a controller's method returns void (i.e., is async handling res object), then by default send a 204 status which makes the controller code throw the headers sent exception.

Looking at the original PR to support async controller methods, the server only sends a response or sets a status code if there is a defined result returned by the controller method, but that assumption was broken in this commit. @pkeuter reported that this commit broke functionality for them in the issue.

What do you think would be the best way to handle async controllers here? Perhaps one solution would be to explicitly provide some configuration to InversifyExpressServer at construction time to allow void return values from controller methods, e.g. (modified sample from above):

// invoke controller's action
const value = await childContainer.getNamed<any>(TYPE.Controller, controllerName)[key](...args);

if (value instanceof HttpResponseMessage) {
    ...
} else if (!res.headersSent && !config.allowVoidControllerMethods) {
    if (value === undefined) {
        res.status(204);
    }
    res.send(value);
}

I'm happy to whip up a PR if someone can please give me some guidance. Thanks!

[johnheroy]

@dcavanagh friendly ping :)

[yotewach]

Does it fix ?