Handling of 405 errors ? (express-openapi)

arnaudruffin commented 4 years ago

Hi!

We have just started using express-openapi and are facing an issue: we can't find a way to handle 405 (method not allowed) errors...

Whenever we try to send a request with a http verb not declared in our apiDoc, we get a 404 with a body like following: `<!DOCTYPE html>

Error

Cannot PATCH /something

` Is it supposed to be handled by express-openapi framework? Are we missing something in the configuration? (we didn't find anything in the test suites) We couldn't find a workaround because this error is not even sent to the errorMiddleware we set in our ExpressOpenAPIArgs... Any help would be welcomed! (... and we are ready to contribute if something needs to be done)

jsdevel commented 4 years ago

@arnaudruffin have you tried adding an error handler?

arnaudruffin commented 4 years ago

@jsdevel yes I have tried to set an error handler but couldn't succeed :(

arnaudruffin commented 4 years ago

To be more specific : I added an error middleware by using the errorMiddleware option when initializing express-openapi, but this middleware isn't triggered when I try to reach one of my path with the wrong HTTP verb. I've also tried to add a global error handler but nothing get triggered.

I've seen that the error I'm getting is what happen when express routing is not configured with all https://stackoverflow.com/questions/51741383/nodejs-express-return-405-for-un-supported-method...

But more important than my use case: do you consider that 405 should be handled by express-openapi?. Because we can work on a workaround, or even contribute to express-openapi, but for now it is unclear for us if it is a misconfiguration of express-openapi on our side or if it is a missing feature :)

jsdevel commented 4 years ago

@aravindanve i think your issue has more to do with express than it does this project per say. what happens if you just initialze an express app (no open-api at all) and make the same request? do you get the same response?

arnaudruffin commented 4 years ago

Thanks @jsdevel for the feedback.

If I initialize an empty express app if will get the same behavior... but i would know how to handle it (by using all after registering routes.) When I use express-open API I don't handle the route registration and I don't know how to override this behaviour.

Once again, my question is more do you consider that 405 should be handled by express-openapi? I would say yes, because it validates that any query match a given open-api spec. But it is only my point of view. If you also think so I can have a look at express-openapi and contribute. But I don't want to spend time on it if you don't think this framework should handle this type of errors.

Here is a quick sandbox just to illustrate this issue : https://github.com/arnaudruffin/express-openapi-issue-677

arnaudruffin commented 4 years ago

@jsdevel For now, my workaround is to "explore" all registered route in express after initializing express-openapi, and use all to return a 405 error:

initialize(openApiArgs);
const authorizedPathAndMethods: Record<string, [string]> = {};
for (const r of expressApp._router.stack.filter((r: { route: any; }) => r.route)) {
    if (authorizedPathAndMethods[r.route.path]) {
        authorizedPathAndMethods[r.route.path].push(Object.keys(r.route.methods)[0].toUpperCase());
    } else {
        authorizedPathAndMethods[r.route.path] = [Object.keys(r.route.methods)[0].toUpperCase()];
    }
}
for (const r of expressApp._router.stack.filter((r: { route: any; }) => r.route)) {
    expressApp.all(r.route.path, (req: any, res: any, next: any) =>
        res.header("allow", authorizedPathAndMethods[r.route.path].join(", "))
            .status(405)
            .send({
                code: 61,
                message: "Method not allowed",
                description: "The URI does not support the requested method.",
            }));
}

If you think that this should be handled by express-openapi, I am still open for a PR?

jsdevel commented 4 years ago

@arnaudruffin this feels more like something that should be handled outside of open-api to me personally, unless you can find something in the open api spec on how unregistered routes/methods should be handled?

kogosoftwarellc / open-api

Handling of 405 errors ? (express-openapi) #677