klein icon indicating copy to clipboard operation
klein copied to clipboard

catch function mapping overwriting existing endpoint

Open sunkencity opened this issue 8 years ago • 3 comments

The most common error I run into when rapidly making a little klein app is when I accidentally overwrite a function when I copy an existing route or accidentally name it the same, could we catch this like in Flask?


@route('/')
def home(request):
    return 'Hello, world!'
    
@route('/foobar')
def home(request):
    return 'Hello, foobar!'

run("localhost", 8080)

[-] Site starting on 8080

vs

app = Flask(__name__)

@app.route("/")
def home():
    return "Hello World!"

@app.route('/foobar')
def home():
    return 'Hello, foobar!'
        
if __name__ == "__main__":
    app.run()

AssertionError: View function mapping is overwriting an existing endpoint function: home

sunkencity avatar Dec 14 '16 09:12 sunkencity

From the documentation of handle_errors() in app.py:

If more than one error handler is registered, the handlers will be executed in the order in which they are defined, until a handler is encountered which completes successfully. If no handler completes successfully, L{twisted.web.server.Request}'s processingFailed() method will be called.

You may have to modify Klein.route() to achieve assertions on duplicated routes.

alexpatel avatar May 08 '17 06:05 alexpatel

This makes metaprogramming with Klein exceedingly error-prone as well. (All of my routes end up being handled by a function called "decorator" if I want to add any functionality on to base Klein). I've tripped over it dozens of times, and it was a caveat we had to make very loudly at our tutorial at PyCon.

cc @wsanchez @moshez since we talked about this somewhat recently

glyph avatar Aug 23 '17 23:08 glyph

This seems more like a problem in the design of how routes are keyed in Klein:

kwargs.setdefault('endpoint', f.__name__)

This makes the name of the function or method subtly important, while IMHO the name of the function shouldn't matter (and shouldn't break things).

I think this could be solved simply by changing that line to something like:

kwargs.setdefault('endpoint', '.'.join([f.__module__, f.__name__])

or perhaps even:

kwargs.setdefault('endpoint', "{}_{}".format(f.__name__, id(f))

This would give every function a unique endpoint identifier, and (please correct me if I am wrong) fit more closely to the spirit of what one would expect to happen.

My assumption here is that a different method is handling an action, it is a different endpoint, even if it shares a name with some method in another module.

trenton42 avatar Nov 29 '17 15:11 trenton42