routing-controllers icon indicating copy to clipboard operation
routing-controllers copied to clipboard

fix: check if headersSent

Open Leechael opened this issue 2 years ago • 7 comments

Description

Check response.headersSent in both handleSuccess & handleError, prevents that response already sent and Express app crashed.

Leechael avatar Feb 17 '22 08:02 Leechael

While I agree with this, I feel like we should somehow indicate that your response from the controller will not be delivered. Simply refusing to return your response from the controller might cause a lot of confusion and debugging. @NoNameProvided thoughts?

attilaorosz avatar Feb 17 '22 09:02 attilaorosz

In my case, I'm create a middleware prevents long-run response (for example, if the API not response in 5 seconds, just send 504 then don't delivery the long-run response).

Without the check the handleSuccess / handleError will failed and crashed.

Leechael avatar Feb 17 '22 09:02 Leechael

I get your point and I agree with it, my only concern is that it might cause unexpected results. We should somehow indicate (log?) that this happened. For example just adding and error.log letting the dev know that the controller result is ignored because the headers are already sent. Obviously console.log is not the perfect solution, just a low hanging fruit :)

attilaorosz avatar Feb 17 '22 09:02 attilaorosz

I'm agree to improve that, how about optional warning to console.log? and the warning and opt-out.

Leechael avatar Feb 17 '22 09:02 Leechael

Lets go with a warning now if the headers are already sent.

attilaorosz avatar Feb 18 '22 19:02 attilaorosz

Could you please add a tests?

attilaorosz avatar Mar 31 '22 05:03 attilaorosz

Could you please add a tests?

Sure. Let me check how to do that.

Leechael avatar Mar 31 '22 06:03 Leechael