apigee-127 / swagger-node-runner

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

override swagger express html 404 error #149

Closed djcuvcuv closed 4 years ago

djcuvcuv commented 4 years ago

hi everyone, i've noticed that when a swagger route is not found, swaggerExpress throws its own HTTP 404 Not Found HTML page, rather than letting my app's global error handler process the error and return a JSON response body.

in other words, i create my swagger-express instance, i register my app with swagger-express, but my app.use(errorHandlerFunction) is never called, e.g.

const app = express();
SwaggerExpress.create(appConfig, function (err, swaggerExpress) {
      // misc code etc.
      app.use(errorHandler); // this middleware is properly called!
      // misc code etc.
      swaggerExpress.register(app); // 404 html page seems to be generated somewhere within this function
      app.use(errorHandler); // but now here this middleware is never called!
});

please note that in ALL other error scenarios in our app/api, the error handler is called and errors are returned as expected. it is only in this case wherein the swagger route is not defined that this mysterious canned html page is returned.

is there any way to override this html page? thanks very much for your help!

best, chris

djcuvcuv commented 4 years ago

@whitlockjc Sorry to bug you; this is not likely in your purview but since you'd helped me diagnose esoteric node/api issues a while ago, i thought i would take a stab and reach out. thanks regardless!

whitlockjc commented 4 years ago

All errors should be sent downstream so you could register an error handler and override it.

djcuvcuv commented 4 years ago

All errors should be sent downstream so you could register an error handler and override it.

@whitlockjc Perhaps im missing something, but i noticed in your language you used the word "register." Does this imply that I can employ swaggerExpress.regiser(myErrorHandler) rather than what I currently do, e.g. app.use(myErrorHandler)? Thanks again for the help here.

I ask this because the app.use(myErrorHandler) (as shown above in the original post) does not ever get called if the line of code is after the swaggerExpress.register(app) line.

whitlockjc commented 4 years ago

It might be due to the order in which you do things, and the fact you're registering your error handler twice. I'd remove the first one and only register the one after swaggerExpress.register(app) and see if that fixes things. But I guess if the Swagger middleware is called, it should send any errors it gets downstream to whatever handlers are after it. There may be an error in one of the middlewares, and I might remember the security middleware from SwaggerExpress having a bug. I can't remember though. If your next test doesn't work, I'll take a deeper dive.

djcuvcuv commented 4 years ago

@whitlockjc I tried using only the app.use(errorHandler); line after the swaggerExpress.register(app); but it still does not seem to ever get called. As a sanity check i stuck a console log statement in the 1st line of the error handler and confirmed that it does not even get called for route not found cases, but again it is called for all other validation/runtime errors.

whitlockjc commented 4 years ago

After some digging, it looks like this is how Express works, 404 responses are not treated as errors so the error handler won't get invoked: https://expressjs.com/en/starter/faq.html#how-do-i-handle-404-responses It would seem that you need to register just a standard app.use(function (req, res, next) { ... to handle this.

djcuvcuv commented 4 years ago

@whitlockjc rookie mistake on my part. thanks for your help!