Skip to content

Conversation

@karthikaluri
Copy link

@karthikaluri karthikaluri commented Jan 24, 2026

Improves handling of peer-side trade failures in Bisq Easy open trades.

Safely handles null and blank peer error messages

Adds a fallback message when no details are available

Ensures consistent popup behavior for expected vs unexpected peer failures

Prevents duplicate popups using DontShowAgainService

No protocol or persistence changes.
Desktop UI only.

Fixes #4205

Summary by CodeRabbit

  • Bug Fixes
    • Improved error message handling and display fallbacks when peer communication errors occur during trades.
    • Enhanced error message sanitization with better default messaging when detailed error information is unavailable.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 24, 2026

Walkthrough

The pull request refactors error message handling in TradeStateController.java by introducing a guarded flow pattern. Error messages now pass through a dontShowAgainService check before display, and fallback to a "noDetails" resource when blank, improving error presentation for trade failures.

Changes

Cohort / File(s) Summary
Error Message Handling Refactor
apps/desktop/desktop/src/main/java/bisq/desktop/main/content/bisq_easy/open_trades/trade_state/TradeStateController.java
Introduced guarded flow for peersErrorMessage observer with early null return and dontShowAgainService integration. Added fallback logic to substitute "noDetails" resource when message is blank. Updated applyStateInfoVBox to use the new displayPeersErrorMessage variable with sanitization and fallback handling.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A guardian flow now shields the way,
Error messages refined each day,
With fallbacks bright and noDetails true,
The trades speak clear, no longer blue!
Sanitized whispers, kindly shown—
Better errors, gracefully grown! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main changes: fixing peer error popup handling and adding fallback for empty error messages.
Linked Issues check ✅ Passed The changes directly address issue #4205 objectives: implementing a user-friendly error popup for peer failures and handling blank/null error messages with fallbacks.
Out of Scope Changes check ✅ Passed All changes are scoped to improving peer error message handling in TradeStateController, directly aligned with issue #4205 requirements.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@HenrikJannsen
Copy link
Contributor

@axpoems Could you review that?

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.

Improve handling of new version enforcement

2 participants