Repository navigation
fix(recaptcha): demo views answered 500 instead of their JSON error body - #14635
Open
rootkiller6788 wants to merge 2 commits into
Open
rootkiller6788 wants to merge 2 commits into
rootkiller6788 wants to merge 2 commits into
Conversation
`except ValueError or Exception as e` reads like it catches both, but `or`
returns the first truthy operand, so the handler only ever caught
ValueError. Anything else escaped it: the assessment call raises
google.api_core errors (PermissionDenied, InvalidArgument, ...) which are
not ValueError, and a request body without "token" raises KeyError before
the call is even made.
All four submit views were written to answer with
{"data": {"error_msg": ...}} in that case, so they were returning a 500
error page instead. Exception already covers ValueError, so the tuple is
not needed.
Signed-off-by: rootkiller6788 <rootkiller6788@users.noreply.github.com>
product-auto-label
Bot
added
samples
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request simplifies exception handling in several endpoints of the recaptcha enterprise demosite by replacing the incorrect 'except ValueError or Exception as e' syntax with 'except Exception as e'. The review feedback correctly points out that using 'str(e.dict)' to serialize the exception will result in an empty dictionary string for standard Python exceptions, and suggests using 'str(e)' instead to properly capture and return the error message.
rootkiller6788
marked this pull request as ready for review
October 13, 2026 14:53
str(e.__dict__) serializes the exception's attribute dictionary, which is
'{}' for exceptions that carry no attributes (e.g. KeyError from a payload
without "token"), and a noisy internal dict for google.api_core errors.
The demo's error_msg should be the exception's message, so use str(e).
Addresses the review comment on GoogleCloudPlatform#14635.
Signed-off-by: rootkiller6788 <rootkiller6788@users.noreply.github.com>
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
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.
Found this while reading the reCAPTCHA Enterprise demosite.
All four submit handlers end with
except ValueError or Exception as e. That doesn't do what it looks like:orreturns the first truthy operand, so the handler only ever catchesValueError. Everything else goes straight past it and the endpoint answers with a 500 error page instead of the{"data": {"error_msg": ...}}it was written to return.Easy to hit in practice - the assessment call raises google.api_core errors like PermissionDenied/InvalidArgument, which aren't ValueError, and a request body without "token" raises KeyError before the call is even made.
Changed the four handlers to
except Exception(Exception already covers ValueError, so no tuple needed).How I checked: pointed
create_assessmentat something that raises PermissionDenied and drove the views with Flask's test client. Before: all four returned 500. After: all four return 200 with the JSON error body. Same for a payload missing "token".One thing I noticed but left alone to keep the diff on the actual bug:
str(e.__dict__)on the next line gives an empty dict for errors that carry no attributes, soerror_msgcan come out as{}.Signed-off-by: rootkiller6788 rootkiller6788@users.noreply.github.com