search

apigee-127 / swagger-node-runner

The heart of Swagger-Node
MIT License
102 stars 124 forks source link

middleware chain is broken when found ctx.output #72

Closed osher closed 7 years ago

osher commented 7 years ago

I'm not sure about this - I need your eyes here. If I'm on target - I'll add tests so you can merge.

.

I'm using restify, and I have other mw on the chain after the swagger_router. (e.g server.on('after', restify.auditLogger(options.auditLogCfg)); )

I first noted that when I dont use json_error_handler - I get the audit entry on requests that error, and when I do - I dont get the desired log entries. (e.g. - the 'after' event is not emitted - that suggests that the mw chain is broken)

Then I noted that they are emitted with defaultErrorHandler and are not emitted with json_error_handler because it creates ctx.output, where defaultErrorHandler does not.

The research led me to the corrected lines. Let me know what you think.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 96.486% when pulling ac6dad9fb5071bddd7a95c32cc38e9cc15c57228 on osher:patch-4 into 13498c97cd65cca7fcbbf092c8c99a986231c18b on theganyo:master.

osher commented 7 years ago

Scott - please take a look here...

Let me know what you think

theganyo commented 7 years ago

I think you're right. I believe the assumption was that this was the terminator on the chain. But even so, I don't believe the appropriate thing is to skip the next() call.

osher commented 7 years ago

that's great. When can we expect 7.0.1?