Can't set headers after they are sent.
Natashkinsasha opened this issue · 8 comments
Hi. I decided to try an example. But on the request I get 204 code and an error in the logs. How I understand it before my error handler, some other one works.
@controller('/foo')
class FooController implements interfaces.Controller {
@httpGet('/')
private index(req: express.Request, res: express.Response, next: express.NextFunction) {
return next(new Error());
}
}
const container = new Container();
const server = new InversifyExpressServer(container);
server.setErrorConfig((app) => {
app.use((err: Error, req: express.Request, res: express.Response, next: express.NextFunction) => {
res.status(500).send('Something broke!');
});
});
const app = server.build();
app.listen(3000);
Error: Can't set headers after they are sent.
at validateHeader (_http_outgoing.js:491:11)
at ServerResponse.setHeader (_http_outgoing.js:498:3)
at ServerResponse.header (E:\workplace\wx-meteor-v2\node_modules\inversify-express-utils\node_modules\express\lib\response.js:767:10)
at ServerResponse.contentType (E:\workplace\wx-meteor-v2\node_modules\inversify-express-utils\node_modules\express\lib\response.js:595:15)
at ServerResponse.send (E:\workplace\wx-meteor-v2\node_modules\inversify-express-utils\node_modules\express\lib\response.js:145:14)
at app.use (E:\workplace\wx-meteor-v2\src\index.ts:37:25)
at Layer.handle_error (E:\workplace\wx-meteor-v2\node_modules\inversify-express-utils\node_modules\express\lib\router\layer.js:71:5)
at trim_prefix (E:\workplace\wx-meteor-v2\node_modules\inversify-express-utils\node_modules\express\lib\router\index.js:315:13)
at E:\workplace\wx-meteor-v2\node_modules\inversify-express-utils\node_modules\express\lib\router\index.js:284:7
at Function.process_params (E:\workplace\wx-meteor-v2\node_modules\inversify-express-utils\node_modules\express\lib\router\index.js:335:12)
We are also having this issue. It appears to only affect the last listed method in the controller class.
I have found 2 workarounds but these go against the documentation of calling return next(err);.
Workaround 1 (throw the error, seems a bit pointless try/catch to throw, but you may want to manipulate the error before you send it up):
try {
const val = await serviceCallAsync();
res.status(200).send(val);
} catch(err) {
throw err;
}
Workaround 2 (utilise the Promises):
try {
const val = await serviceCallAsync();
Promise.resolve(val);
} catch(err) {
return Promise.reject(err);
}
We haven't been able to figure out where the problem lies but it could be something to do with the decorators.
@PodaruDragos I have very recently been added as maintainer and I am going to start working through the backlog
Will there be an actual solution? I found that throwing an error works fine but not calling next
Any update on this? The error handler doesn't seems to be working for me too.
I'm having the same issue. I have this configuration to handle some method error with setErrorConfig and sometimes the server responds with status 204 No content, instead of the actual json and status. It always happens with the same endpoint, and with most of the others endpoints works fine, returning the corresponding error status
inversifyServer.setErrorConfig((app) => {
app.use((err, req, res, _next) => {
}
// const jsonResponse ={ ... }
return res.status(err.status || 500).json(jsonResponse);
});
@httpPost(
'/foo',
'SomeMiddleware',
)
async Foo(req: Request, res: Response, next: NextFunction) {
try {
// do something
return res.status(201).json();
} catch (err) {
return next(err);
}
}
The error in the console is Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client
It looks like the response is sent before accessing the setErrorConfig callback.
I'm having a super weird issue as well with this...
To give some context,
This is my implementation of overriding the default error handler:
export const errorHandler = (err: Error, req: Request, res: Response, next: NextFunction) => {
//We check if the error is coming from our custom error objects.
if (err instanceof HttpException) {
return res.status(err?.statusCode).json({
message: err?.message,
errorName: err?.name,
stack: err?.stack,
});
}
//Errors that a unexpected and not handled by our custom errors.
return res.status(500).json({
message: err?.message,
errorName: err?.name,
stack: err?.stack,
});
};
I have the following endpoint, let's call it endpoint1
@httpPost('/refreshAccessToken')
public async refreshAccessToken(req: Request, res: Response, next: NextFunction): Promise<any> {
try {
if (!req.body.refreshToken || !req.body.accessToken || !req.body.id || !req.body.email) {
throw new BadRequestError('Missing fields')
}
return res.status(200).json({status: 200,message: 'Success getting new access token with refresh token.',});
} catch (error) {
next(error);
}
}
and then just below I have this empty endpoint, let's call this endpoint2.
@httpPost('')
public async() {
}
If I have both endpoint1 and endpoint2, everything works fine, endpoint1 will throw a BadRequestError which will get caught in the catch block and then will be delegated to the errorHandler middleware.
As soon as I remove endpoint2, I get a 204 no-content and the following message:
ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client
I have no idea what is happening...
I confirm the reports from @nref-dan and @Mannydheer
The error occurs for the last method listed in the controller.
Would be great to see some updates on this problem
The Real Issue & Solution
This issue is quite straightforward to resolve but comes with a philosophical twist. I tackled this in the ExpressoTS Framework, where you can seamlessly use hbs, ejs, express-handlebars, etc. While I could submit a PR, I’ve chosen not to—this is more of a philosophical battle I’d prefer to avoid.
The Issue
In Express.js, if you don’t explicitly end the response (by calling methods like res.send, res.json, res.end, etc.), the request will hang. This leaves it up to the developers to handle these cases. For example:
app.get("/", (req, res) => {
return;
});In inversify-express-utils the author of the lib decided to provide a solution for these use cases, which honestly, is a great idea. The author call in their code and tests "Falsey" return;
The idea behind automatically handling undefined return values with a 204 No Content status in inversify-express-utils is to prevent this hanging by default. There are few issues with his approach in terms of logic, and the way that the metadata is being handle to distinguish what is res.send, res.render and any other method with their unique needs, thus its indeed interfering with the way res.render works.
The file and method responsible for generating this situation is server.ts and the method is handlerFactory
Here is the code:
private handlerFactory(
controllerName: string,
key: string,
parameterMetadata: ParameterMetadata[],
): RequestHandler {
return async (
req: Request,
res: Response,
next: NextFunction
): Promise<void> => {
try {
const args = this.extractParameters(req, res, next, parameterMetadata);
const httpContext = this._getHttpContext(req);
httpContext.container.bind<HttpContext>(TYPE.HttpContext)
.toConstantValue(httpContext);
// invoke controller's action
const value = await (httpContext.container.getNamed<Controller>(
TYPE.Controller,
controllerName,
)[key] as ControllerHandler)(...args);
if (value instanceof HttpResponseMessage) {
await this.handleHttpResponseMessage(value, res);
} else if (instanceOfIHttpActionResult(value)) {
const httpResponseMessage = await value.executeAsync();
await this.handleHttpResponseMessage(httpResponseMessage, res);
} else if (value instanceof Function) {
value();
} else if (!res.headersSent) {
if (value === undefined) {
res.status(204);
}
res.send(value);
}
} catch (err) {
next(err);
}
};
}When you use res.render with or without return the value is undefined, always falling in to setting the response to 204 blocking the possibility to render the page right after set the header.
if (value === undefined) {
res.status(204);
}
res.send(value);SOLUTION
Option 1 - Remove the "Falsey" validation and make sure that the inversify-express-utils is aligned with expressjs default behavior
private handlerFactory(
controllerName: string,
key: string,
parameterMetadata: ParameterMetadata[],
): RequestHandler {
return async (
req: Request,
res: Response,
next: NextFunction
): Promise<void> => {
try {
const args = this.extractParameters(req, res, next, parameterMetadata);
const httpContext = this._getHttpContext(req);
httpContext.container.bind<HttpContext>(TYPE.HttpContext)
.toConstantValue(httpContext);
// invoke controller's action
const value = await (httpContext.container.getNamed<Controller>(
TYPE.Controller,
controllerName,
)[key] as ControllerHandler)(...args);
if (value instanceof HttpResponseMessage) {
await this.handleHttpResponseMessage(value, res);
} else if (instanceOfIHttpActionResult(value)) {
const httpResponseMessage = await value.executeAsync();
await this.handleHttpResponseMessage(httpResponseMessage, res);
} else if (value instanceof Function) {
value();
} else if (!res.headersSent) {
if (value !== undefined) {
res.send(value);
}
}
} catch (err) {
next(err);
}
};
}Secret sauce is right here in case you haven't see it
} else if (!res.headersSent) {
if (value !== undefined) {
res.send(value);
}Option 2 - Make sure value can have a metadata or any data that can distinguish the method utilized in order to treat different those methods and their unique usecases.
Final considerations
If you decide to clone and build the lib by yourself, with the fix I use, pay attention to one particular test case in the serve.test.ts which validates "Falsey" return. This test obviously will fail
it('should work for controller methods who\'s return value is falsey', done => {
@controller('/user')
class TestController {
@httpDelete('/')
public async delete(): Promise<void> {
return new Promise((resolve) => setTimeout(resolve, 100));
}
}
server = new InversifyExpressServer(container);
void supertest(server.build())
.delete('/user')
.expect(204, '', done);
});
});I hope this sheds some light on the final solution for those trying to use render engines with inversify and express. Cheers!