finalhandler cannot 404 after headers sent +0ms about routing-controllers

[pleerock]

provide a code snippet of what you are doing

[MContagious]

Please find the following git repo with reproduceble code.
https://github.com/MContagious/routing-controller-sample

[pleerock]

what will happen if you remove @EmptyResultCode(404) ?

[MContagious]

Initially I dont have that annotation. Later I included to resolve that error but it is still throwing 404 error. But, The browser is getting the json response. Error is written in the server log.

[pleerock]

use new @OnUndefined / @OnNull decorators to resolve this issue.

[NoNameProvided]

This happening to me as well. I try to handle a file upload I see this in the console:

  body-parser:json content-type "multipart/form-data; boundary=--------------------------042144984244033328614529" +977ms
  body-parser:json skip parsing +0ms
  morgan log request +977ms
POST /upload/photos 200 - ms - 14
  finalhandler cannot 404 after headers sent +977ms

And it also happens when I implicitly return a value:

@Post('/photos')
  async uploadPhotos() {
    return { wee: 'wooo' };
  }

In this case the console output is:

  express:router dispatching POST /upload/photos +34s
  express:router query  : /upload/photos +0ms
  express:router expressInit  : /upload/photos +0ms
  body-parser:json content-type "application/json" +34s
  body-parser:json content-encoding "identity" +0ms
  body-parser:json read body +0ms
  body-parser:json parse body +1ms
  body-parser:json parse json +0ms
  express:router logger  : /upload/photos +1ms
  express:router jsonParser  : /upload/photos +1ms
  body-parser:json body already parsed +1ms
  morgan log request +34s
POST /upload/photos 200 - ms - 34
  finalhandler cannot 404 after headers sent +34s

Probably next() gets called even after a route match?

[pleerock]

Probably next() gets called even after a route match?

yes, it is

[NoNameProvided]

So is it an easy fix? I think it happens somewhere in the routing-controllers package. Or is it intentional?

[pleerock]

Or is it intentional?

yes next() method call is intentional, because otherwise middleware that execute after your controllers wont work.

[NoNameProvided]

Yes, but if I see correctly all we have to do is to register a special last middleware after registering the normal ones not?

I see you register them in index.ts at here , isn't it enough to change this to something like

new RoutingControllers(driver)
        .initialize()
        .registerMiddlewares("before")
        .registerControllers(controllerClasses)
        .registerMiddlewares("after", middlewareClasses); 
        .registerFinalMiddleware(FinalMiddleware);

where final middleware looks something like:

import {ExpressMiddlewareInterface} from "../../src/driver/express/ExpressMiddlewareInterface";
import {Middleware} from "../../src/decorator/Middleware";

@Middleware({ type: "after" })
export class FinalMiddleware implements ExpressMiddlewareInterface {

    use(request: any, response: any, next?: Function): any {
     // next() not called here 
   }
    
}

and the registerFinalMiddleware function looks something like:

registerFinalMiddleware(class: Function): this {
        this.metadataBuilder
            .buildMiddlewareMetadata([class])
            .forEach(middleware => this.driver.registerMiddleware(middleware));

        return this;
    }

[NoNameProvided]

Or we can just do

new RoutingControllers(driver)
        .initialize()
        .registerMiddlewares("before")
        .registerControllers(controllerClasses)
        .registerMiddlewares("after", [...middlewareClasses, FinalMiddleware]);

but then we still have to ensure metadata is already built for this class.

[NoNameProvided]

Btw shouldn't we reopen this? Having "Cannot set headers after response is sent" errors for every request is not okay, and I believe using @OnUndefined / @OnNull doesn't solve the problem, but my proposal does.

[pleerock]

Having "Cannot set headers after response is sent" errors for every request is not okay

right, its not okay. And I don't have such errors after every request. Can you show what are you doing that you have such error?

@Post('/photos')
  async uploadPhotos() {
    return { wee: 'wooo' };
  }

This code should not give you such error. As I understood you have such error because of finalhandler you are using, right? If yes, then why are you using it?

[NoNameProvided]

As I understood you have such error because of finalhandler you are using, right? If yes, then why are you using it?

finalhandler is a dependency of the express framework itself. It is called as a last middleware to set the status code to500 if you call next() too many times.

On the user side it can be fixed with using a custom middleware:

import { Middleware, ExpressMiddlewareInterface } from "routing-controllers";
import { NextFunction, Request, Response } from 'express';

const debug = require('debug')('middleware:FinalMiddleware');

@Middleware({ type: "after" })
export class FinalMiddleware implements ExpressMiddlewareInterface {

  use(req: Request, res: Response, next?: NextFunction): any {
    debug('Final middleware reached!')
  }

}

and loading it as the last one:

this.express = createExpressServer({
      routePrefix: '',
      middlewares: [CorsMiddleware, FinalMiddleware],
    });

This code should not give you such error.

The only cause I can think of is that I am loading some vanilla express middleware this way:

this.express = createExpressServer({
      routePrefix: '',
      middlewares: [CorsMiddleware, FinalMiddleware],
    });
this.express.use(logger('dev'));
this.express.use(bodyParser.json());

But it should still cause the error with the above solution if this would be the root of the error. So I don't have any idea how finalHandler get reached. I dont call next() anywhere in my code except in CorsMiddleware which has the type before

[pleerock]

There is a next() after each route execution, can your issue be because of #114 ?

[NoNameProvided]

I don't think so because I have no specific route handlers for OPTION requests, I just early return with a 200 code and an empty response body in the CORS middleware (type: before) and I don't call next at all in that case so my routes are not reached.

[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.