fix: Gateway Telegram error replies crash on non-ASCII exception text (fixes #1752)#1754
Conversation
- Create safe error message helper in unicode_utils.py - Update Telegram bot error handling to use ASCII-safe formatting - Fix gateway server error handler to extract root cause errors - Ensure UTF-8 encoding for YAML config file reads - Add support for accented character conversion to ASCII Fixes #1752 π€ Generated with [Claude Code](https://claude.ai/code) Co-authored-by: praisonai-triage-agent[bot] <praisonai-triage-agent[bot]@users.noreply.github.com>
|
@coderabbitai review |
|
/review |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more β On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
β Actions performedReview triggered.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the βοΈ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
π WalkthroughWalkthroughThis PR fixes Windows encoding crashes in the Telegram gateway error handler by introducing Unicode safety utilities, enabling UTF-8 YAML loading, and applying safe error text transformation when exceptions are sent back to users. Exception text containing non-ASCII characters (emoji, warnings, international text) no longer crashes the error handler itself. ChangesWindows Telegram error encoding safety
Estimated code review effortπ― 3 (Moderate) | β±οΈ ~20 minutes Suggested reviewers
Poem
π₯ 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 |
|
@copilot Do a thorough review of this PR. Read ALL existing reviewer comments above from Qodo, Coderabbit, and Gemini first β incorporate their findings. Review areas:
|
Greptile SummaryThis PR fixes Unicode encoding crashes in the gateway Telegram bot error handlers on Windows, where the default console encoding (cp1252) cannot represent characters like
Confidence Score: 5/5Safe to merge β the change is narrowly scoped to error handler formatting and YAML file reads, with no impact on the agent execution path. The sanitization logic is correct: exception text is converted to pure ASCII before being sent to Telegram, which directly addresses the cp1252 crash. The YAML encoding fix is standard. The only observation is that greedy quantifiers in a few regex patterns could produce longer-than-intended fragments from multi-occurrence error strings, but this does not affect correctness or safety of the bot reply. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant User as Telegram User
participant Handler as handle_message()
participant Agent as AI Agent
participant UU as unicode_utils
participant TG as Telegram API
User->>Handler: sends message
Handler->>Agent: run agent
alt success
Agent-->>Handler: response text
Handler->>TG: reply_text(response)
TG-->>User: message delivered
else exception (e.g. OpenAI 429 with β symbol)
Agent-->>Handler: raises Exception(e)
Handler->>UU: safe_log_message(e)
UU-->>Handler: UTF-8 safe string
Handler->>Handler: logger.error(...)
Handler->>UU: extract_root_cause_from_error(str(e))
UU->>UU: match _API_PATTERNS / keywords
UU-->>Handler: simplified error text
Handler->>UU: safe_error_message(user_error)
UU->>UU: replace symbols, NFKD normalize, ASCII-only
UU-->>Handler: pure ASCII string
Handler->>TG: reply_text("Error: " + ascii_text)
TG-->>User: clean error message
end
Reviews (2): Last reviewed commit: "fix: address reviewer feedback on unicod..." | Re-trigger Greptile |
|
@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Focus ONLY on Python packages (praisonaiagents, praisonai). Do NOT modify praisonai-rust or praisonai-ts. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding. Phase 1: Review per AGENTS.md
Phase 2: FIX Valid Issues Phase 3: Final Verdict |
|
Claude finished @MervinPraison's task ββ View job Final Architecture Review βConducted comprehensive architectural review per AGENTS.md guidelines and addressed all valid reviewer feedback. Todo List
Architecture Compliance ββ
Protocol-driven core: Changes correctly placed in wrapper package ( β Minimal scope: Fix addresses specific Windows Unicode crash without scope creep or feature additions β Backward compatible: Zero API changes, purely internal error handling improvements β Performance: No hot-path regressions, fixed performance issue identified by reviewers Issues Fixed1. Logic Issue - Double Sanitization
2. Code Quality - Dead Code Comments
3. Testing
Files Modified
Final Verdict: β APPROVEDThis PR is architecturally sound and ready for merge:
The PR successfully resolves issue #1752 while maintaining architectural integrity and addressing all review feedback. Ready to merge. Commit pushed: |
- Remove dead code comment and simplify safe_log_message implementation - Fix double sanitization in extract_root_cause_from_error by returning raw text - Improve code comments for clarity Addresses Greptile review feedback on performance and logic issues.
Summary
Fixes Unicode encoding crashes in gateway Telegram bot error handlers on Windows where the default console encoding is cp1252 instead of UTF-8.
Problem
When a gateway Telegram bot encounters an exception containing non-ASCII characters (like β warning symbols from OpenAI API errors), the error handler crashes with:
This masks the real underlying error (e.g., OpenAI 429 quota exceeded) and confuses users.
Solution
1. Created
unicode_utils.pyhelper module:safe_error_message(): Converts Unicode exceptions to ASCII-safe text for bot repliessafe_log_message(): Preserves Unicode for UTF-8 logs while preventing crashesextract_root_cause_from_error(): Extracts meaningful errors from complex error chains2. Updated error handlers:
3. Fixed YAML config loading:
encoding="utf-8"Test Results
β Unicode safety tests passed
β Encoding scenario tests passed
β Safe logging tests passed
Before/After
Before (Windows cp1252):
After (all platforms):
Testing
The fix handles various Unicode scenarios:
Closes #1752
π€ Generated with Claude Code
Summary by CodeRabbit