feat: Add custom exceptions#847
Conversation
|
@Mr-Sunglasses is attempting to deploy a commit to the sparckles Team on Vercel. A member of the Team first needs to authorize it. |
for more information, see https://pre-commit.ci
CodSpeed Performance ReportMerging #847 will not alter performanceComparing Summary
|
sansyrox
left a comment
sansyrox
left a comment
There was a problem hiding this comment.
Hey @Mr-Sunglasses ,
Thanks for the PR. I have some suggestions :D
for more information, see https://pre-commit.ci
Thanks, @sansyrox 🙏🏻, for the suggestions, I've fixed then 😄 |
|
Hey @Mr-Sunglasses 👋 Could you please add some integration tests? And the CI is also failing for some reason. |
sansyrox
force-pushed
the
main
branch
5 times, most recently
from
07f28de to
0b766c9
Compare
![recurseml[bot]](https://avatars.githubusercontent.com/in/1054195?s=60&v=4){kind=small}
| class HTTPException(Exception): | ||
| def __init__(self, status_code: int, detail: str | None = None) -> None: | ||
| if detail is None: | ||
| detail = http.HTTPStatus(status_code).phrase |
There was a problem hiding this comment.
The code attempts to create an HTTPStatus instance with an unchecked status_code, which can raise ValueError for invalid codes (like values above 599 or negative numbers). This exception is not caught, leading to unexpected behavior when an invalid status code is provided. The code should either validate the status_code range first or catch ValueError and provide a fallback message.
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
|
😱 Found 1 issue. Time to roll up your sleeves! 😱 |
sansyrox
force-pushed
the
main
branch
3 times, most recently
from
5fb08e0 to
0472e42
Compare
sansyrox
left a comment
sansyrox
left a comment
There was a problem hiding this comment.
Thanks for this, @Mr-Sunglasses, and apologies it sat so long. The landscape has shifted since you opened it: robyn/exceptions.py now already lives in main (it came in with the CORS PR #1019), which is why this is conflicting. That said, you've spotted a real gap — those exceptions still aren't exported from the top-level package, so from robyn import HTTPException doesn't actually work today. The most useful path forward would be to rebase and trim this down to just the export wiring in __init__.py (adding the import + the __all__ entries) plus your nice docs example, dropping the duplicate exceptions.py. Heads up that this also overlaps with #951, so let's coordinate to avoid double work — and a small test in test_exception_handling.py would seal it.
sansyrox
added
reviewed
2 participants
Description
This PR fixes #550