`Error: Can't set headers after they are sent.` when using `next()` in production about express-oas-generator HOT 19 CLOSED

@kiprasmel In my opinion this is not exactly an issue of express-oas-generator but a consequence of using it. Let me explain:

Just reproduced it on the examples by running server_advanced.js and executing GET /students/stranger request via swagger-ui. Here's the thing: To execute handleRequests at the end of each request we always need to call next() after responding, as it's always the last middleware.

But what happens with the paths that are matched by more than one app.use selector? (Eg: GET /students/stranger and GET /students/:id). So consider the following options:

1. Reply and not calling next() to avoid the subsequent middlewares to be executed (response won't be documented).
2. Check res.headersSent on the subsequent matched middlewares and then recalling next(). Check this post.

Let me know if it helps!

from express-oas-generator.

@maxiejbe, bravo, excellent investigation! @kiprasmel could you confirm that it is the case?

from express-oas-generator.

Need some more time to investigate/understand the "original issue".

from express-oas-generator.

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:

• use swagger-ui-express/swagger-ui-dist module directly, same way as it is done in index.js:143
• add production mode to express-oas-generator so it intercepts nothing but just serves SwaggerUI and specification
• deploy separately Swagger UI web site (it contains only static assets: HTMLs, CSSs and JSs) to CDN for your company with specifications to your services

I would like to emphasize one more time: express-oas-generator generates 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.

from express-oas-generator.

@maxiejbe Yep, it indeed is.

Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client
    at ServerResponse.setHeader (_http_outgoing.js:518:11)
    at exports.openAPIDocsHTMLHandler (/home/kipras/projects/turbo-schedule/server/src/route/v1/openAPIDocs.ts:69:7)
    at Layer.handle [as handle_request] (/home/kipras/projects/turbo-schedule/node_modules/express/lib/router/layer.js:95:5)
    at trim_prefix (/home/kipras/projects/turbo-schedule/node_modules/express/lib/router/index.js:317:13)
    at /home/kipras/projects/turbo-schedule/node_modules/express/lib/router/index.js:284:7
    at Function.process_params (/home/kipras/projects/turbo-schedule/node_modules/express/lib/router/index.js:335:12)
    at next (/home/kipras/projects/turbo-schedule/node_modules/express/lib/router/index.js:275:10)
    at /home/kipras/projects/turbo-schedule/node_modules/express/lib/router/index.js:635:15
    at next (/home/kipras/projects/turbo-schedule/node_modules/express/lib/router/index.js:260:14)
    at next (/home/kipras/projects/turbo-schedule/node_modules/express/lib/router/route.js:127:14)
Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client

currently, as mentioned above ☝ #36 (comment), 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.

from express-oas-generator.

@maxiejbe

In my opinion this is not exactly an issue of express-oas-generator but a consequence of using it

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.

from express-oas-generator.

@maxiejbe

Please take a look at this fix: a2aa633 +1

Yep, that's indeed the fix 🎉 I suppose we can close this, or perhaps add this note to the documentation beforehand?

from express-oas-generator.

@maxiejbe

Please take a look at this fix: a2aa633 +1

Yep, that's indeed the fix 🎉 I suppose we can close this, or perhaps add this note to the documentation beforehand?

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.

from express-oas-generator.

@maxiejbe : this was most helpful! 👏

from express-oas-generator.

@sarpik Are you using any middleWare? Like SwaggerUI?
Also, can you show us your app.js/main.js/index.js?

from express-oas-generator.

@Nathan961207 yes, I'm using middleware.

Everything's available in my project - https://github.com/sarpik/turbo-schedule - specifically the server/ folder.

server.ts: https://github.com/sarpik/turbo-schedule/blob/master/server/src/server.ts

the routes are available in server/src/route/v1

If you have further questions - feel free to ask!

from express-oas-generator.

@sarpik I took a look at https://github.com/sarpik/turbo-schedule/blob/master/server/src/util/applyAPIDocsGenerator.ts and saw this:

    if (process.env.NODE_ENV === "production") {
		/**
		 * this SHALL NOT be used in production!
		*/

	return;
    }

Is this done to avoid having to type

(process.env.NODE_ENV !== "production")

in your routes?
Otherwise, I would suggest removing any middleware you are using. That was my issue when generating a swagger.json.

from express-oas-generator.

Hey @Nathan961207, thanks for taking a look.
This is done to avoid invoking expressOasGenerator.init so that it's not applied in production - so that there's no /api-docs endpoint and no route handlers are added from express-oas-generator.

You'd still have to use

if (process.env.NODE_ENV !== "production") {
	return next()
}

in the routes, because calling next() after you've modified the responses body (like res.json({ foo: "bar" })) should never be done (stackoverflow.com/a/7789131, stackoverflow.com/a/7086621).
But I do it in development to generate the documentation and because no errors are produced in development mode, but once you run the app in production, the errors do start coming up on every request, hence why I use the !== "production" check.

By the way, this issue originally was that I need to use next() in places where it shouldn't be used (after modifying the response's body/headers etc).

By the way, I do not have a problem with generating the openapi.json docs there are some problems, I'll create a separate issue later - I just run the server/script/generateOpenAPIDocs.ts with ts-node-dev and both the openAPI.json and my modified version, openAPI.lean.json get generated, from which I use the openAPI.lean.json.

What middleware did you disable to generate the swagger.json?
For me, it works as it is currently.

I should note that the endpoint /api/v1/docs becomes available after a openAPI.lean.json file gets created by server/script/(generateOpenAPIDocs|modifyGeneratedOpenAPIDocs).ts, but the regular file openAPI.json is always served in development mode at the route /api-docs by express-oas-generator anyways.

My main concern in this issue is the Error: Can't set headers after they are sent. when using next() in production.

from express-oas-generator.

@sarpik I did disable SwaggerUI to get it working. Then i refactor the code so SwaggerUI was setup before express-oas-generator middleware.

I am sure you can call next() after modifying the body because you have not sent the header back.
Here is an important snippet from a previous issue: #24 (comment)

Ok, here is the problem: express-oas-generator (EOG) adds parameters handler as a very last middleware. If any middleware or path in router breaks the chain and doesn't pass execution to next middleware/router then very last EOG middleware won't be called. So call next() or next(err) as the very last line in your handler.

from express-oas-generator.

@Nathan961207 oh nevermind I found what was causing the inability to generate the openapi spec - I just needed to place the expressOasGenerator.init(app) right after creating the express app (const app = express()). More info here: #32 (comment)

This solves the second issue, but this original one (req headers can't be set after they're sent) - I'm still not sure about. I'll do some testing soon and will report back here.

Thank you for your help by the way, Nathan:)

from express-oas-generator.

@mpashkovskiy Yeah I'm currently using express-oas-generator only in development mode - it's disabled in production

Once I'm deploying my application, I run the tests and thus the documentation gets generated. Then I serve it with redoc, similarly to swagger-ui-dist.

I'm not afraid of potential security issues, because I use a fake database and only test with the data I code into my tests, meaning that no real data is used.

And the performance is fine, because I do not use express-oas-generator in production - I only serve the already generated specification.

I was concerned about the error

Error: Can't set headers after they are sent.

which happens to occur only in production mode, even though the express-oas-generator is not enabled; only because of (I suspect) the next() calls, which I avoid in production by using a condition, as mentioned above:

if (process.env.NODE_ENV !== "production") {
	return next()
}

with it, no errors occur. Without it, they do.

The error is my only concern at the moment, as I don't have any other problems.

from express-oas-generator.

@kiprasmel Is this still happening? Would you create an example to try it and include it on the roadmap? Ty!

from express-oas-generator.

@kiprasmel Please take a look at this fix: a2aa633 👍

from express-oas-generator.

Sounds like a plan.

from express-oas-generator.