Skip to content

Fix RPC error message regression in reqwest client#971

Merged
DanGould merged 1 commit into
payjoin:masterfrom
bc1cindy:fix/rpc-error-message-regression-966
Aug 19, 2025
Merged

Fix RPC error message regression in reqwest client#971
DanGould merged 1 commit into
payjoin:masterfrom
bc1cindy:fix/rpc-error-message-regression-966

Conversation

@bc1cindy

Copy link
Copy Markdown
Contributor

The migration from bitcoincore-rpc to reqwest caused a regression where RPC error messages became generic "RPC request failed with status: 500" instead of showing which method failed and the actual error from Bitcoin Core.

Changes:

  • Include RPC method name in error messages for better debugging
  • Extract and display actual error message from JSON-RPC error response
  • Replace generic HTTP status errors with descriptive Bitcoin Core messages
  • Add comprehensive tests with real Bitcoin Core integration

Before: "RPC request failed with status: 500 Internal Server Error"
After: "RPC 'walletcreatefundedpsbt' failed: Insufficient funds"

Fixes #966

Pull Request Checklist

Please confirm the following before requesting review:

  • A human has reviewed every single line of this code before opening the PR (no auto-generated, unreviewed LLM/robot submissions).
  • I have read CONTRIBUTING.md and rebased my branch to produce hygienic commits.

The migration from bitcoincore-rpc to reqwest caused a regression where
RPC error messages became generic "RPC request failed with status: 500"
instead of showing which method failed and the actual error from Bitcoin Core.

Changes:
- Include RPC method name in error messages for better debugging
- Extract and display actual error message from JSON-RPC error response
- Replace generic HTTP status errors with descriptive Bitcoin Core messages
- Add comprehensive tests with real Bitcoin Core integration

Before: "RPC request failed with status: 500 Internal Server Error"
After:  "RPC 'walletcreatefundedpsbt' failed: Insufficient funds"

Fixes payjoin#966
@bc1cindy
bc1cindy force-pushed the fix/rpc-error-message-regression-966 branch from 2399550 to 9e01d6b Compare August 19, 2025 01:52
@coveralls

Copy link
Copy Markdown
Collaborator

Pull Request Test Coverage Report for Build 17057359385

Details

  • 56 of 56 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 86.37%

Totals Coverage Status
Change from base Build 17051959649: 0.1%
Covered Lines: 7788
Relevant Lines: 9017

💛 - Coveralls

@bc1cindy
bc1cindy marked this pull request as ready for review August 19, 2025 01:57

@DanGould DanGould left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Although I agree with @arminsabouri that this should be replaced with alpen's client soon this PR does address the regression. Much appreciated quick fix.

ACK 9e01d6b

Comment thread payjoin-cli/src/app/rpc.rs
@DanGould
DanGould merged commit fc55f59 into payjoin:master Aug 19, 2025
10 checks passed
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.

Fix RPC error message regression

3 participants