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

fix: correct error handling in connect method #41

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

2wheeh
Copy link

@2wheeh 2wheeh commented Dec 3, 2024

Breaking Changes

N/A

Changes

  • fix: correct error handling in connect method

Associated Issues

closes #40

Description

  1. Currently, the reject call in the subscription callback is handled by useConnect's catch block, bypassing WalletConnectModalSign.connect's error handling
  2. This causes the approval promise chain to remain pending after Modal closed error
  3. When a subsequent connection attempt succeeds, the lingering approval promise from the previous attempt consumes this success
  4. This leaves the second attempt's promise unresolved, keeping subscribing modal
  5. When the previous resolved promise reaches finally, it closes the modal, and the subscription from the subsequent attempt throws a Modal closed error

To fix this, I consolidated the approval promise and modal closure handling within the connect method using Promise.race, ensuring that either a successful connection or modal closure properly resolves/rejects the promise chain.

After

2024-12-03.11.38.30.mov

@2wheeh
Copy link
Author

2wheeh commented Dec 4, 2024

@Cali93 @ganchoradkov Hi could I ask you to review this?

@2wheeh
Copy link
Author

2wheeh commented Dec 8, 2024

@Cali93 @ganchoradkov kind reminder here 👀

@2wheeh
Copy link
Author

2wheeh commented Dec 19, 2024

@Cali93 @ganchoradkov Hi! Another very kine reminder to review this.

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.

[bug] modal close error causes subsequent connection attempts to fail
1 participant