Skip to content

fix: improve error handling in GitHub Copilot provider#919

Merged
xiaket merged 1 commit intosipeed:mainfrom
CZH-THU:fix/improve-github-copilot-error-handling
Mar 1, 2026
Merged

fix: improve error handling in GitHub Copilot provider#919
xiaket merged 1 commit intosipeed:mainfrom
CZH-THU:fix/improve-github-copilot-error-handling

Conversation

@CZH-THU
Copy link
Contributor

@CZH-THU CZH-THU commented Feb 28, 2026

📝 Description

This PR improves error handling in the GitHub Copilot provider by:

  1. Fixing ignored error: The SendAndWait method's error return value was previously ignored using _. Now it's properly handled and wrapped in a descriptive error message.

  2. Improving error message: Enhanced the error message for the unimplemented stdio mode to provide clearer guidance to users, suggesting they use grpc mode instead.

  3. Adding documentation: Added a TODO comment with a reference link to help future contributors implement the stdio mode.

These changes improve code reliability and user experience by ensuring errors are properly propagated and users receive helpful guidance.

🗣️ Type of Change

  • 🐞 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 📖 Documentation update
  • ⚡ Code refactoring (no functional changes, no api changes)

🤖 AI Code Generation

  • 🤖 Fully AI-generated (100% AI, 0% Human)
  • 🛠️ Mostly AI-generated (AI draft, Human verified/modified)
  • 👨‍💻 Mostly Human-written (Human lead, AI assisted or none)

🔗 Related Issue

📚 Technical Context (Skip for Docs)

🧪 Test Environment

  • Hardware: PC
  • OS: Windows 10
  • Model/Provider: GitHub Copilot (not tested with actual provider, code review only)
  • Channels: N/A (provider-level change)

📸 Evidence (Optional)

Click to view Logs/Screenshots

Before:

resp, _ := session.SendAndWait(ctx, copilot.MessageOptions{
    Prompt: string(fullcontent),
})

After:

resp, err := session.SendAndWait(ctx, copilot.MessageOptions{
    Prompt: string(fullcontent),
})

if err != nil {
    return nil, fmt.Errorf("failed to send message to copilot: %w", err)
}

☑️ Checklist

  • My code/docs follow the style of this project.
  • I have performed a self-review of my own changes.
  • I have updated the documentation accordingly (added TODO comment with reference).
  • Code passes linting checks (verified with read_lints).
  • Commit message follows Conventional Commits format.

Copy link

@nikolasdehor nikolasdehor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: fix: improve error handling in GitHub Copilot provider

Clean, focused fix. All three changes are improvements.

Analysis

  1. SendAndWait error handling -- The original resp, _ := session.SendAndWait(...) silently discarded errors. This is a real bug -- if the Copilot session fails (network timeout, auth issue, rate limit), the code would proceed with a nil resp and hit the "empty response" error instead of the actual failure reason. The fix properly propagates the error with context ("failed to send message to copilot: %w").

  2. Improved stdio error message -- Adding "please use 'grpc' mode instead" is helpful user guidance. The TODO with reference link is good for future contributors.

  3. Error wrapping -- Using %w for error wrapping is correct Go convention, allowing callers to use errors.Is/errors.As for error inspection.

Minor Notes

  1. The resp == nil check after the new error check is still valuable as a defensive guard (SDK could theoretically return nil response without error).

  2. No test changes, but this is a provider that is hard to unit test without mocking the Copilot SDK. The fix is mechanical and safe.

LGTM. Straightforward correctness improvement.

Copy link
Collaborator

@xiaket xiaket left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@xiaket
Copy link
Collaborator

xiaket commented Mar 1, 2026

@CZH-THU please reformat the file(make fix will do it for you) and let me know.

- Fix ignored error from SendAndWait call
- Improve error message for unimplemented stdio mode with helpful guidance
- Add TODO comment with reference link for future stdio implementation
@CZH-THU CZH-THU force-pushed the fix/improve-github-copilot-error-handling branch 2 times, most recently from 7aa7579 to f05db6d Compare March 1, 2026 11:52
@xiaket xiaket merged commit 71bdeb4 into sipeed:main Mar 1, 2026
4 checks passed
vvr3ddy added a commit to vvr3ddy/picoclaw that referenced this pull request Mar 1, 2026
Changes from upstream:
- fix(channel): config cleanup and regex precompile (sipeed#911, sipeed#916)
- fix(github_copilot): improve error handling (sipeed#919)
- fix(wecom): context leaks and data race fixes (sipeed#914, sipeed#918)
- fix(tools): HTTP client caching and response body cleanup (sipeed#940)
- feat(tui): Add configurable Launcher and Gateway process management (sipeed#909)
- feat(migrate): Update migration system with openclaw support (sipeed#910)
- fix(skills): Use registry-backed search for skills discovery (sipeed#929)
- build: Add armv6 support to goreleaser (sipeed#905)
- docs: Sync READMEs and channel documentation
@Orgmar
Copy link
Contributor

Orgmar commented Mar 2, 2026

@CZH-THU Properly handling the ignored error in SendAndWait and improving the error messages for the GitHub Copilot provider is a solid improvement. Silent errors are always a pain to debug.

We have a PicoClaw Dev Group on Discord for contributors to share ideas and collaborate. If you're interested, send an email to support@sipeed.com with the subject [Join PicoClaw Dev Group] CZH-THU and we'll send you the invite link.

@CZH-THU
Copy link
Contributor Author

CZH-THU commented Mar 2, 2026

@CZH-THU Properly handling the ignored error in SendAndWait and improving the error messages for the GitHub Copilot provider is a solid improvement. Silent errors are always a pain to debug.

We have a PicoClaw Dev Group on Discord for contributors to share ideas and collaborate. If you're interested, send an email to support@sipeed.com with the subject [Join PicoClaw Dev Group] CZH-THU and we'll send you the invite link.

Sure,Thanks!

liangzhang-keepmoving pushed a commit to liangzhang-keepmoving/picoclaw that referenced this pull request Mar 2, 2026
- Fix ignored error from SendAndWait call
- Improve error message for unimplemented stdio mode with helpful guidance
- Add TODO comment with reference link for future stdio implementation
hyperwd pushed a commit to hyperwd/picoclaw that referenced this pull request Mar 5, 2026
- Fix ignored error from SendAndWait call
- Improve error message for unimplemented stdio mode with helpful guidance
- Add TODO comment with reference link for future stdio implementation
Pluckypan pushed a commit to Pluckypan/picoclaw that referenced this pull request Mar 6, 2026
- Fix ignored error from SendAndWait call
- Improve error message for unimplemented stdio mode with helpful guidance
- Add TODO comment with reference link for future stdio implementation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants