refactor: rely on global exception handlers in route modules - #289
Open
emiliano-go wants to merge 3 commits into
Open
emiliano-go wants to merge 3 commits into
emiliano-go wants to merge 3 commits into
Conversation
…ute try/except Route handlers in user, api_keys, rate_limit, and tier modules repeated the same handle_exception boilerplate that diverged from the global error policy (some api_keys routes leaked str(e) in 404/403 details, and 500s used an ad-hoc message). The global DomainError handler and CatchAllErrorMiddleware registered in app_factory already cover every exception type these blocks handled, so routes now let exceptions propagate. Generic 500 wording is unified on GENERIC_ERROR_MESSAGE.
Services raise UserNotFoundError instead of returning None, so the 'if user is None' 404 branches were dead code. delete_user_account now verifies permission before the user lookup, matching update_user_profile and avoiding username enumeration via 404-vs-403 responses.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Most route handlers repeated the same try/except block that mapped domain exceptions and returned a 500, even though the app already registers a global DomainError handler and a catch-all middleware with the same mapping. The copies had also diverged: several api_keys routes leaked raw exception text in 404 and 403 responses, and two different generic 500 messages coexisted.
This PR removes the per-route boilerplate from the user, api_keys, rate_limit and tier routers and lets exceptions propagate to the global handlers. It also removes dead
if user is None404 checks (services raise UserNotFoundError instead of returning None) and makes delete_user_account check permissions before the user lookup, matching update_user_profile; this intentionally changes the response for deleting a non-existent user from 404 to 403 so the API does not reveal which usernames exist.Behavior note: 403 and 404 responses sourced from domain errors now return the generic message with a support id instead of specific details, which is the policy the global handler already enforced.
Depends on #284 (read-endpoint-auth), which touches the same route files; merge that one first.
Written by Kimi, Authored by @emiliano-go