Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve exception msg and handling when 'Same Bank' BIC/SWIFT blank #7258

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

cparke2
Copy link
Contributor

@cparke2 cparke2 commented Sep 24, 2024

Users in most countries (see BankUtil.useValidation() for the list of exceptions) can create a "Transfer Same Bank" payment account in Bisq with no BIC/SWIFT code. However, to create an offer using this account type, in most countries (see BankUtil.isBankIdRequired() for these exceptions) the account must have a value in the bankId field, or JAVA will throw a 'null pointer exception', and then the Bisq application will then become unhealthy and hang trying on exit. This fix adds a validation check for that condition and throws a more "friendly" exception which explains the problem, and it happens earlier to keep the app. healthy.

Users in most countries (see BankUtil.useValidation() ) can create a "Transfer Same Bank"
account with no BIC/SWIFT code. However, to create an offer using this account, it must
have a bankId or JAVA will throw a 'null pointer exception', and the app. will become
unhealthy and hang trying on exit. This fix adds a validation check for the condition and
throws a more "friendly" exception which explains the problem and keeps the app. healthy.
@cparke2
Copy link
Contributor Author

cparke2 commented Sep 24, 2024

BEFORE:
Screenshot from 2024-09-24 05-55-00

AFTER:
Screenshot from 2024-09-24 05-56-17

@cparke2
Copy link
Contributor Author

cparke2 commented Sep 24, 2024

Another thing I wanted to do, but didn't want to "step on someone's toes", was just make the BIC/SWIFT required on the account form for creating the "Transfer Same Bank" payment method. Or, make it use Bank Name rather than BIC for bank matching ("Transfer Specific Banks" works differently already and uses Bank Name for all currencies to match offers). So this pull request represents the minimum: just handle the invalid case more gracefully; hopefully that'll be enough info for the user to fix their problem.

I also opened a discussion on these related account types which raises other issues with this payment method and it's sibling:
https://github.com/bisq-network/bisq/discussions/7256

Copy link
Collaborator

@HenrikJannsen HenrikJannsen left a comment

Choose a reason for hiding this comment

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

utACK

@HenrikJannsen HenrikJannsen merged commit 68f5625 into bisq-network:master Sep 26, 2024
3 checks passed
@alejandrogarcia83 alejandrogarcia83 added this to the v1.9.18 milestone Nov 5, 2024
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.

3 participants