fix: capture exception details and fix inconsistent error response fields#47
fix: capture exception details and fix inconsistent error response fields#47shruthipavalavel wants to merge 1 commit intoOWASP-BLT:mainfrom
Conversation
📊 Monthly LeaderboardHi @shruthipavalavel! Here's how you rank for March 2026:
Scoring this month (across OWASP-BLT org): Open PRs (+1 each), Merged PRs (+10), Closed (not merged) (−2), Reviews (+5; first two per PR in-month), Comments (+2, excludes CodeRabbit). Run |
owasp-blt
bot
added
unresolved-conversations: 0
needs-peer-review
|
👋 Hi @shruthipavalavel! This pull request needs a peer review before it can be merged. Please request a review from a team member who is not:
Once a valid peer review is submitted, this check will pass automatically. Thank you!
|
WalkthroughException handling in two API endpoint handlers was improved to capture and return actual error details instead of generic messages. Explanatory comments were removed, and file formatting was adjusted. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/index.py`:
- Around line 116-119: The exception-handling paths currently reuse
response_headers (which contains "Cache-Control: public, max-age=30") when
returning Response.new, causing error responses with exception details to be
publicly cached; update the except block(s) that call Response.new (referencing
Response.new and response_headers) to use a non-cacheable header set — e.g.,
clone response_headers then override "Cache-Control" to "no-store" (and
optionally add "Pragma: no-cache" and "Expires: 0") — and apply the same change
to the other error return around lines 171-175 so all error responses containing
str(e) are not cached.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| except Exception as e: | ||
| return Response.new( | ||
| json.dumps({'balance': None, 'error': 'Internal error'}), | ||
| json.dumps({'balance': None, 'error': str(e)}), | ||
| {'status': 500, 'headers': response_headers} |
There was a problem hiding this comment.
Don’t cache 500 responses that now include exception details.
Line 119 and Line 174 reuse response_headers (Cache-Control: public, max-age=30) for exception paths. Since error now includes str(e), these failures can be publicly cached and replayed to other clients for 30s. Use non-cacheable headers on error responses.
Suggested patch
@@
except Exception as e:
+ error_headers = {
+ **cors_headers,
+ 'Content-Type': 'application/json',
+ 'Cache-Control': 'no-store',
+ }
return Response.new(
json.dumps({'balance': None, 'error': str(e)}),
- {'status': 500, 'headers': response_headers}
+ {'status': 500, 'headers': error_headers}
)
@@
except Exception as e:
+ error_headers = {
+ **cors_headers,
+ 'Content-Type': 'application/json',
+ 'Cache-Control': 'no-store',
+ }
return Response.new(
json.dumps({'supply': None, 'error': str(e)}),
- {'status': 500, 'headers': response_headers}
+ {'status': 500, 'headers': error_headers}
)Also applies to: 171-175
🧰 Tools
🪛 Ruff (0.15.7)
[warning] 116-116: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/index.py` around lines 116 - 119, The exception-handling paths currently
reuse response_headers (which contains "Cache-Control: public, max-age=30") when
returning Response.new, causing error responses with exception details to be
publicly cached; update the except block(s) that call Response.new (referencing
Response.new and response_headers) to use a non-cacheable header set — e.g.,
clone response_headers then override "Cache-Control" to "no-store" (and
optionally add "Pragma: no-cache" and "Expires: 0") — and apply the same change
to the other error return around lines 171-175 so all error responses containing
str(e) are not cached.
github-project-automation
bot
moved this from Backlog
to Ready
in 📌 OWASP BLT Project Board
owasp-blt
bot
added
unresolved-conversations: 1
and removed
unresolved-conversations: 0
labels
|
|
1 participant
Fixes #46
What this PR does
Two bugs fixed in src/index.py:
Bug 1: Silent exception handling
Both handle_sol_balance and handle_token_supply were catching exceptions without capturing the error message, returning a generic "Internal error" with no debugging information.
Changed
except Exceptiontoexcept Exception as eand returnstr(e)so the actual error is surfaced in the response.Bug 2: Inconsistent error response field
The except block in handle_token_supply was returning
'balance': Noneinstead of'supply': None, which was inconsistent with its success response format.Changes
Summary by CodeRabbit