Skip to content

fix(cli): improve error handling for disabled payment channels#13350

Merged
rjan90 merged 3 commits intomasterfrom
phi/fix-cli-info-err
Oct 6, 2025
Merged

fix(cli): improve error handling for disabled payment channels#13350
rjan90 merged 3 commits intomasterfrom
phi/fix-cli-info-err

Conversation

@rjan90
Copy link
Copy Markdown
Contributor

@rjan90 rjan90 commented Sep 19, 2025

Proposed Changes

Register ErrPaymentChannelDisabled in the RPC error registry so it properly survives RPC serialization. This allows errors.Is() to work correctly on the client side without needing string comparison fallbacks.

Changes:

  • Add errPaymentChannelDisabled type and register it in api/api_errors.go
  • Update node/impl/paych to use api.ErrPaymentChannelDisabled
  • Simplify cli/info.go to only use errors.Is() check
  • Remove PaymentChannelDisabledMessage constant (no longer needed)

Checklist

Before you mark the PR ready for review, please make sure that:

@github-project-automation github-project-automation Bot moved this to 📌 Triage in FilOz Sep 19, 2025
@rjan90 rjan90 force-pushed the phi/fix-cli-info-err branch from 29191f1 to 18793ab Compare September 19, 2025 15:38
…o command

fix(cli): improve error handling for disabled payment channels in info command
@rjan90 rjan90 force-pushed the phi/fix-cli-info-err branch from 18793ab to fe92fd7 Compare September 19, 2025 15:39
@rjan90 rjan90 moved this from 📌 Triage to 🔎 Awaiting Review in FilOz Sep 23, 2025
@rjan90 rjan90 marked this pull request as ready for review September 23, 2025 06:49
Copilot AI review requested due to automatic review settings September 23, 2025 06:50
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves error handling in the lotus info command for disabled payment channels to prevent automation script failures. The change ensures the command exits with code 0 instead of code 1 when payment channels are disabled by default.

  • Updated error message text for ErrPaymentChannelDisabled to be more concise
  • Enhanced error detection logic to use both errors.Is() and string matching for robustness

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
node/impl/paych/paych.go Updated error message text for ErrPaymentChannelDisabled to be more concise
cli/info.go Added dual error detection using both errors.Is() and string matching to handle disabled payment channels

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread cli/info.go Outdated
Comment thread cli/info.go Outdated
- Add PaymentChannelDisabledMessage constant to avoid magic strings
- Update CLI error handling to use the constant instead of hardcoded string
- Improves maintainability and reduces coupling between error message and logic
@rjan90 rjan90 added the skip/changelog This change does not require CHANGELOG.md update label Sep 23, 2025
@BigLep BigLep requested a review from rvagg September 23, 2025 13:52
@rvagg
Copy link
Copy Markdown
Member

rvagg commented Sep 23, 2025

We should just be able to use the errors.Is for this but the catch is that the error needs to traverse the RPC boundary, we do that by registering error types in api/api_errors.go so both sides of the RPC know to encode it with a number, and decode it from that number, then it appears on the client side as the same error type and errors.Is works. Then we can do away with the string constant exported and just put the string into the error itself.

@rjan90 rjan90 changed the title fix(cli): improve error handling for disabled payment channels in inf… fix(cli): improve error handling for disabled payment channels Sep 30, 2025
refactor: use RPC-traversable error for disabled payment channels
@rjan90
Copy link
Copy Markdown
Contributor Author

rjan90 commented Sep 30, 2025

Updated to traverse the RPC boundary in: 37f7e5e

@github-project-automation github-project-automation Bot moved this from 🔎 Awaiting Review to ✔️ Approved by reviewer in FilOz Oct 6, 2025
@rjan90 rjan90 merged commit 9f34758 into master Oct 6, 2025
98 checks passed
@rjan90 rjan90 deleted the phi/fix-cli-info-err branch October 6, 2025 04:32
@github-project-automation github-project-automation Bot moved this from ✔️ Approved by reviewer to 🎉 Done in FilOz Oct 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip/changelog This change does not require CHANGELOG.md update

Projects

Status: 🎉 Done

Development

Successfully merging this pull request may close these issues.

3 participants