fix: handle ignored io.ReadAll errors across codebase#1142
fix: handle ignored io.ReadAll errors across codebase#1142mengzhuo merged 7 commits intosipeed:mainfrom
Conversation
io.ReadAll errors were silently discarded with `body, _ := io.ReadAll(...)`, which could cause empty or partial data to be used for JSON unmarshaling or error messages. This adds proper error checks for all instances.
There was a problem hiding this comment.
Pull request overview
This PR improves reliability of HTTP response handling by ensuring io.ReadAll(...) errors are not silently ignored, preventing downstream logic from operating on empty/partial bodies (e.g., JSON parsing, error reporting).
Changes:
- Replace
body, _ := io.ReadAll(...)withbody, err := io.ReadAll(...)across multiple packages. - Add early returns with contextual errors when reading response bodies fails.
- Improve error-path diagnostics by ensuring error responses are read (when possible) before formatting error messages.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/providers/antigravity_provider.go | Adds io.ReadAll error handling for Antigravity API responses. |
| pkg/channels/wecom/bot.go | Adds io.ReadAll error handling in webhook reply error path. |
| pkg/channels/wecom/app.go | Adds io.ReadAll error handling for WeCom app upload/send error responses. |
| pkg/channels/wecom/aibot.go | Adds io.ReadAll error handling for response_url failure bodies. |
| pkg/channels/line/line.go | Adds io.ReadAll error handling for LINE API error responses. |
| pkg/auth/oauth.go | Adds io.ReadAll error handling for OAuth device/token flows. |
| cmd/picoclaw/internal/auth/helpers.go | Adds io.ReadAll error handling for Google userinfo fetch. |
| cmd/picoclaw-launcher/internal/server/auth_handlers.go | Adds io.ReadAll error handling for Google userinfo fetch. |
| cmd/picoclaw-launcher-tui/internal/ui/model.go | Adds io.ReadAll error handling when reading limited error responses in model test UI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot open a new pull request to apply changes based on the comments in this thread |
sipeed-bot
bot
added
type: bug
Combine both shadow variable fix (readErr) and proper error classification (ClassifySendError) in wecom app and bot channels.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
huaaudio
left a comment
huaaudio
left a comment
There was a problem hiding this comment.
Solid PR that handles ignored error catching. LGTM
fix: handle ignored io.ReadAll errors across codebase
fix: handle ignored io.ReadAll errors across codebase
📝 Description
io.ReadAll errors were silently discarded with
body, _ := io.ReadAll(...), which could cause empty or partial data to be used for JSON unmarshaling or error messages. This adds proper error checks for all instances.🗣️ Type of Change
🤖 AI Code Generation
🔗 Related Issue
📚 Technical Context (Skip for Docs)
🧪 Test Environment
📸 Evidence (Optional)
Click to view Logs/Screenshots
☑️ Checklist