swagger-node-runner icon indicating copy to clipboard operation
swagger-node-runner copied to clipboard

204 No Content

Open osher opened this issue 8 years ago • 3 comments

There's a codepath I'm not comfortable with, I need your take on this.

from W3 spec:

The 204 response MUST NOT include a message-body, and thus is always terminated by the first empty line after the header fields.

So, in contrast, IMHO - the case slips our code here: https://github.com/theganyo/swagger-node-runner/blob/master/lib/connect_middleware.js#L87

the problem is that if ctx.output is falsful - including zero (0), empty string (""), null, undefined, and obviously false - the respondse.end() is not called, where the last in chain-of-responsibility is usually the framework, wich ends with it's default response, usually 404 in the form of cannot GET /the-path.

All of the above (except undefined maybe) are legit replies the server may be required to provide, regardless to 204 status code, where there should be cases where the server should emit no body.

Am doing something wrong or missing anything?

If I'm not missing anything - LMK, and I'll start a PR to fix this...

osher avatar Nov 24 '16 15:11 osher

Yes, I agree. It shouldn't be relying on a falsy condition... it should be more specific than that.

theganyo avatar Nov 24 '16 20:11 theganyo

IMHO, when a request is mapped to a swagger-operation - it should not leave the pipe without emitting a response.

Am I missing something? Can you think of a case where this assumption fails?

If so - then there should not be such a condition. I mean - we can argue what translate(body,mime) does with nulls and undefineds, but the more I consider it - I'm convinced: The 404 > Cannot PUT /my-svc/path is not a correct reply from the web-server

I believe that having the request mapped to an operation - response.end() must be called and a response must be provided, be it empty or not Edited: and the next callback should be called

Can we agree on this?

osher avatar Nov 30 '16 11:11 osher

Hmm. Well, the theory was that we're still trying to operate as a good citizen within the framework that exists and not completely take control... so it was following the "do no harm" doctrine and only writing when it was told to. I'm also pretty certain that there are developers that expect to hook in regular connect middleware after the pipe runs.

That said, you're completely right... an error such as you have above makes no sense at all. So the question is: Is it safe to assume that the response can be ended? And.. honestly, I'm not sure. Would it solve the issue to just call write on the response instead of end?

theganyo avatar Nov 30 '16 16:11 theganyo