assure response content-type to match the spec - Githubissues

apigee-127 / swagger-node-runner

The heart of Swagger-Node

MIT License

102 stars 124 forks source link

assure response content-type to match the spec #74

Closed osher closed 7 years ago

osher commented 7 years ago

When the user uses next(null, output) instead of using res.end(..) and does not set the content-type response header - the code path reaches the point where ctx.output is translated according to res.getHeader('content-type').

In this case - it's rendered using util.inspect(..) - which is not the format declared by the operation.

This fix tries the res.getHeaders('content-type') first, and if missing - fallsback to using the first entry on theoperation.produces` as default.

(still need to add tests)

coveralls commented 7 years ago

Coverage decreased (-0.4%) to 96.038% when pulling b83323866d9ea945be6f9b4cbe5f66a3dfabe14a on osher:patch-5 into 13498c97cd65cca7fcbbf092c8c99a986231c18b on theganyo:master.

osher commented 7 years ago

This one is going to be tedious... I added pending tests.

Can you please confirm that I reverse engineered the specs correctly?

coveralls commented 7 years ago

Coverage decreased (-0.4%) to 96.038% when pulling eccebbb5eba5afee512e2274a5c880f5515db225 on osher:patch-5 into 13498c97cd65cca7fcbbf092c8c99a986231c18b on theganyo:master.

coveralls commented 7 years ago

Coverage decreased (-0.4%) to 96.038% when pulling 070472b095e58dfe645f4764b269d0b02b74a290 on osher:patch-5 into d52ff581e677f4dbbfd0c59641ab0d6f94eac8cb on theganyo:master.

osher commented 7 years ago

mmm. maybe you'd prefer a different strategy to test all this. I saw you didn't write tests for connect_adapter - perhaps because you relay on e2e tests for that?

coveralls commented 7 years ago

Coverage decreased (-0.4%) to 95.748% when pulling f83fc7387818992656889d946517c053fbcd36d2 on osher:patch-5 into 6f61ed80c5a6e4db0de329e61a80b5448b316dff on theganyo:master.

coveralls commented 7 years ago

Coverage decreased (-0.4%) to 95.748% when pulling a766a3f450f23e8a8f12e64ff5c0b2567d235bcb on osher:patch-5 into 6f61ed80c5a6e4db0de329e61a80b5448b316dff on theganyo:master.

coveralls commented 7 years ago

Coverage decreased (-0.4%) to 95.748% when pulling 3c8d2ed01c2f65c4e0d87b268e0e2e61fab1e1c7 on osher:patch-5 into 54555fc671437d78fbe27388be76fd581569b41f on theganyo:master.

osher commented 7 years ago

@coveralls are you for real? this commit is from December 2016!

theganyo commented 7 years ago

Maybe coveralls is trying to tell me something. ;) The reason I haven't merged this is that it seems like it could be a breaking change for someone... and I haven't heard anyone else complain about it. Do you have any thoughts on the potential impact on other folks?

osher commented 7 years ago

hmm. What's breaking? The case where user did not send content-type and the infra sends for her the same content-type defined by the user in the open-spec doc? I'd think that's rather intentional...

theganyo commented 7 years ago

LOL. Ok. You're right. I reviewed the code again. I think I was being overly skittish. Thanks for the contrib!