express-oas-generator: `Error: Can't set headers after they are sent.` when using `next()` in production
I used to have route handlers like this:
router.get("/", async (_req, res) => {
try {
return res.json({ foo: "bar" });
} catch (err) {
console.error(err);
return res.status(500).json({ foo: "", error: err });
}
});
and after reading #24 and #29, I’ve started using next:
-router.get("/", async (_req, res) => {
+router.get("/", async (_req, res, next) => {
try {
- return res.json({ foo: "bar" });
+ res.json({ foo: "bar" });
+ return next();
} catch (err) {
console.error(err);
- return res.status(500).json({ foo: "", error: err });
+ res.status(500).json({ foo: "", error: err });
+ return next(err);
}
});
This seems to work fine in development, but when using the API in production, I get errors like this:
Error: Can't set headers after they are sent.
at SendStream.headersAlreadySent (/home/kipras/projects/turbo-schedule/node_modules/send/index.js:390:13)
at SendStream.send (/home/kipras/projects/turbo-schedule/node_modules/send/index.js:617:10)
at onstat (/home/kipras/projects/turbo-schedule/node_modules/send/index.js:729:10)
at FSReqCallback.oncomplete (fs.js:159:5)
If I revert back to how I was using the route handlers without calling next OR only calling next if the response body/headers were NOT modified, everything works fine & I don’t get the errors anymore.
This answer provides some useful information: https://stackoverflow.com/a/7789131
also, from the same thread: https://stackoverflow.com/a/7086621
What solves the issue is only calling next() when the NODE_ENV !== “production”, which makes the code look terrible and I’d love to avoid this:D
router.get("/", async (_req, res, next) => {
try {
res.json({ foo: "bar" });
+ if (process.env.NODE_ENV !== "production") {
- return next();
+ return next();
+ }
+
+ return;
} catch (err) {
console.error(err);
res.status(500).json({ foo: "", error: err });
+ if (process.env.NODE_ENV !== "production") {
- return next(err);
+ return next(err);
+ }
+
+ return;
}
Is there possibly any way to work around this?
I only really use the API spec generation once building:
I start up the server, init expressOasGenerator, activate API routes so that the docs get generated & stop the server.
Then, once I have the openAPI.json file, I serve it in production through another package from npm (I use redoc, one could also use swagger-ui), because the file is just static, it does not need updating etc.
TL;DR:
The errors shouldn’t be there. The next() also should not be called in these places, as noted in the stackoverflow threads above.
Working around with process.env.NODE_ENV works, but it’s just wrong and I don’t want to have my stuff like this.
Once again, I’d love to solve this if possible.
About this issue
- Original URL
- State: closed
- Created 5 years ago
- Comments: 19 (15 by maintainers)
@maxiejbe, bravo, excellent investigation! @kiprasmel could you confirm that it is the case?
@kiprasmel In my opinion this is not exactly an issue of
express-oas-generatorbut a consequence of using it. Let me explain:Just reproduced it on the examples by running
server_advanced.jsand executingGET /students/strangerrequest via swagger-ui. Here’s the thing: To executehandleRequestsat the end of each request we always need to callnext()after responding, as it’s always the last middleware.But what happens with the paths that are matched by more than one
app.useselector? (Eg:GET /students/strangerandGET /students/:id). So consider the following options:next()to avoid the subsequent middlewares to be executed (response won’t be documented).res.headersSenton the subsequent matched middlewares and then recallingnext(). Check this post.Let me know if it helps!
@maxiejbe : this was most helpful! 👏
If you agree, let’s close it and I’ll include a note for this case after updating the
README.md. Just to keep a cleaner roadmap.@maxiejbe
Yep, that’s indeed the fix 🎉 I suppose we can close this, or perhaps add this note to the documentation beforehand?
@maxiejbe
lol, this sentence alone made me realize what’s wrong:D I’ve encountered the issue where more than one handler gets reached, and indeed it’s because we call next, not because we’re using express-oas-generator:D
Great job finding this out - I’ll try the fix real soon, but I’m pretty sure that it’s indeed the case.
@maxiejbe Yep, it indeed is.
currently, as mentioned above ☝ https://github.com/mpashkovskiy/express-oas-generator/issues/36#issuecomment-546595767, I use a work-around to not get these errors in production, and ignore them in development.
The project is available – https://github.com/kiprasmel/turbo-schedule (
/server/), though it’s not a small one, so debugging might be harder. I should try to create a simpler reproducible project, but am afraid I won’t have much time anytime soon, sadly.One note: this module should not be used in production. It may add too much overhead to your service by intercepting each and every request/response. Idea of the module is to generate baseline OpenAPI specification which will be reviewed, manually edited and only after deployed with your service or made available for REST API consumers by other means (swagger generators, etc).
I suppose you would like to use it because it provides Swagger UI for generated specification. There could be several options to get it:
swagger-ui-express/swagger-ui-distmodule directly, same way as it is done in index.js:143express-oas-generatorso it intercepts nothing but just serves SwaggerUI and specificationI would like to emphasize one more time:
express-oas-generatorgenerates baseline specification, it should be reviewed and edited to avoid publishing sensitive information in examples of requests/responses.But I’m not saying that I’m not going to investigate the problem.
Need some more time to investigate/understand the “original issue”.