fix: capture exception details in error responses (#46)#48
fix: capture exception details in error responses (#46)#48hopkdj wants to merge 1 commit intoOWASP-BLT:mainfrom
Conversation
- Changed 'except Exception:' to 'except Exception as e:' in both handle_sol_balance and handle_token_supply - Error responses now include the actual exception message via str(e) instead of the generic 'Internal error' string - Makes debugging API failures much easier Fixes OWASP-BLT#46
📊 Monthly LeaderboardHi @hopkdj! 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 @hopkdj! 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 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 2
🤖 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 181-185: In the except Exception as e block that returns
Response.new, avoid returning str(e); replace the detailed exception text with a
generic error message (e.g., "Internal server error") so no RPC/parsing details
are leaked, while keeping the same response shape {'supply': None, 'error':
<generic message>} and preserving response_headers and status 500; locate the
catch by the except Exception as e and the call to Response.new to apply the
change.
- Around line 124-128: Replace the direct exposure of the exception string in
the Response.new call by logging the full exception server-side (use the
existing logger or add one) and returning a sanitized generic error message to
the client; in the except block that currently captures Exception as e and calls
Response.new with json.dumps({'balance': None, 'error': str(e)}), change it to
log the exception and stack trace and set the response body to a non-sensitive
message (e.g., {'balance': None, 'error': 'Internal server error'}) while
preserving status and response_headers, and optionally gate returning detailed
error text behind a development-only env flag.
🪄 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.
Security risk: Raw exception messages may leak sensitive information.
Returning str(e) directly exposes internal details to API consumers. Exceptions from the fetch() call or JSON parsing could reveal:
- RPC URLs (which may contain API keys if using paid providers like Alchemy/QuickNode)
- Internal file paths or stack trace fragments
- Connection error details with endpoint information
For debugging, log the exception server-side and return a generic message to clients, or sanitize the exception type without the full message.
Proposed fix: sanitize error output
except Exception as e:
+ # Log full exception for debugging (Cloudflare Workers: use console.log or wrangler tail)
+ # print(f"handle_sol_balance error: {e}")
return Response.new(
- json.dumps({'balance': None, 'error': str(e)}),
+ json.dumps({'balance': None, 'error': 'Internal server error'}),
{'status': 500, 'headers': response_headers}
)Alternatively, if exception details are needed for client-side debugging in non-production environments, consider gating this behind an environment flag.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 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} | |
| ) | |
| except Exception as e: | |
| # Log full exception for debugging (Cloudflare Workers: use console.log or wrangler tail) | |
| # print(f"handle_sol_balance error: {e}") | |
| return Response.new( | |
| json.dumps({'balance': None, 'error': 'Internal server error'}), | |
| {'status': 500, 'headers': response_headers} | |
| ) |
🧰 Tools
🪛 Ruff (0.15.7)
[warning] 124-124: 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 124 - 128, Replace the direct exposure of the
exception string in the Response.new call by logging the full exception
server-side (use the existing logger or add one) and returning a sanitized
generic error message to the client; in the except block that currently captures
Exception as e and calls Response.new with json.dumps({'balance': None, 'error':
str(e)}), change it to log the exception and stack trace and set the response
body to a non-sensitive message (e.g., {'balance': None, 'error': 'Internal
server error'}) while preserving status and response_headers, and optionally
gate returning detailed error text behind a development-only env flag.
| except Exception as e: | ||
| return Response.new( | ||
| json.dumps({'supply': None, 'error': 'Internal error'}), | ||
| json.dumps({'supply': None, 'error': str(e)}), | ||
| {'status': 500, 'headers': response_headers} | ||
| ) |
There was a problem hiding this comment.
Same security concern: sanitize exception output.
Apply the same fix here—return a generic error message rather than str(e) to avoid leaking sensitive details from RPC or parsing failures.
Proposed fix
except Exception as e:
+ # Log full exception for debugging
+ # print(f"handle_token_supply error: {e}")
return Response.new(
- json.dumps({'supply': None, 'error': str(e)}),
+ json.dumps({'supply': None, 'error': 'Internal server error'}),
{'status': 500, 'headers': response_headers}
)🧰 Tools
🪛 Ruff (0.15.7)
[warning] 181-181: 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 181 - 185, In the except Exception as e block that
returns Response.new, avoid returning str(e); replace the detailed exception
text with a generic error message (e.g., "Internal server error") so no
RPC/parsing details are leaked, while keeping the same response shape {'supply':
None, 'error': <generic message>} and preserving response_headers and status
500; locate the catch by the except Exception as e and the call to Response.new
to apply the change.
github-project-automation
bot
moved this from Backlog
to Ready
in 📌 OWASP BLT Project Board
owasp-blt
bot
added
unresolved-conversations: 2
and removed
unresolved-conversations: 0
labels
|
|
1 participant
Fixes #46
Changes:
except Exception:toexcept Exception as e:in bothhandle_sol_balanceandhandle_token_supplystr(e)instead of the genericInternal errorstringSummary by CodeRabbit