Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conflicting routes are only detected in validate() or run() #672

Closed
cschreib opened this issue Jun 26, 2023 · 0 comments · Fixed by #676
Closed

Conflicting routes are only detected in validate() or run() #672

cschreib opened this issue Jun 26, 2023 · 0 comments · Fixed by #676
Labels
feature Code based project improvement

Comments

@cschreib
Copy link
Contributor

cschreib commented Jun 26, 2023

The code below does not generate any exception or error in v1.0+5:

CROW_ROUTE(app, "/page1")([]{ return "Hello world 1"; });
CROW_ROUTE(app, "/page1")([]{ return "Hello world 2"; });

It is only later down the line, when calling app.run(), that we get an exception:

"handler already exists for /page1"

This is a bit late, and if the routes are defined in various places, it can be difficult to track the offending CROW_ROUTE. If the error was raised immediately when creating the route, debugging would be easier.

I appreciate there's probably a cost to pay for this check, and that not everyone wants to pay for it on each new added route. An alternative I considered was to explicitly call validate() in my code, after adding each route. But unfortunately crow::Crow only expects validate() to be called once, when all the routes are set up. This is because the validated_ internal flag is never reset to false when adding new routes. If it were, then I could just call validate() whenever I want to do error checking.

I thought this could be an easy fix, but currently validate() does more than just checking, it also sets up the static route and applies the blueprints. These are actions that shouldn't be done multiple times, but they are also not strictly speaking "validation". I think they should probably be separated from the actual route validation, perhaps into a separate function.

@The-EDev The-EDev added the feature Code based project improvement label Jun 29, 2023
@gittiver gittiver linked a pull request Feb 7, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Code based project improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants