falconry / falcon

The no-magic web data plane API and microservices framework for Python developers, with a focus on reliability, correctness, and performance at scale.
https://falcon.readthedocs.io/en/stable/
Apache License 2.0
9.51k stars 937 forks source link

Different behaviour between suffixed responders and "normal" ones #2167

Open Sirtea opened 1 year ago

Sirtea commented 1 year ago

I'm developing a generic resource with all the API operations (GET/POST collection, GET/PUT/PATCH/DELETE element). The idea is to reuse this component by passing a DAO object to make it truly generic. The idea is that all the methods should be disabled by some resources, and I did this with this kind of code (minimal reproduction):

import falcon

class GenericResource:
    def __init__(self, on_get_collection=True):
        if on_get_collection:
            self.on_get_collection = self._on_get_collection

    def _on_get_collection(self, req, resp):
        resp.media = {'message': 'on_get_collection'}

app = falcon.App()
res1 = GenericResource()
res2 = GenericResource(on_get_collection=False)

As I start gunicorn server, my app crashes:

falcon.routing.util.SuffixedMethodNotFoundError: No responders found for the specified suffix

At this point, the resource "res2" is useless because it has no methods, I know... but this happens when I disable all "collection" methods. If I remove the suffix='collection' I only get 405s on every request, like this:

import falcon

class NoneResource:
    pass

app = falcon.App()
app.add_route('/res3', NoneResource())
# the next route crashes...
# app.add_route('/res4', NoneResource(), suffix='collection')

The issue is easy to bypass by removing the "collection" add_route, but I still think that both routes /res3 (normal route) and /res4 (suffixed route) should behave the same way, with no methods available...

vytas7 commented 1 year ago

Hi @Sirtea! I agree it is a somewhat inconsistent behaviour...

However, not sure if we should allow adding empty resources at all? :thinking: HTTP 405 means that a method is not allowed, and the server should return a list of allowed methods, but in this case there are none?

Sirtea commented 1 year ago

In the rare event that I disable all "collection" methods, it makes no sense to have the add_route(..., suffix='collection') statement. Same could be said in the event of disabling all the "normal" methods. Probably is a bug not worth looking at, but I just wanted you to know.

And to be correct, any resource with no responders should be giving a 404 instead of a 405 on all HTTP verbs... Again, it's a weird usage of the resources from a developers perspective, but his own fault.

CaselIT commented 1 year ago

I'm more inclined to think that the behaviour done when suffix is specified, ie the raise, is more correct.

vytas7 commented 2 months ago

I agree with @CaselIT, I think we should model the behaviour after the current one for suffixed resources.