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.

Sign up for GitHub

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

Jump to bottom

Reduce 1 minute replyTimeout when the "get_info" event is called from the executeMultiNip47Request method in NWCClient.ts #295

Open
Dayvvo opened this issue Jan 15, 2025 · 3 comments · May be fixed by #299
Open

Reduce 1 minute replyTimeout when the "get_info" event is called from the executeMultiNip47Request method in NWCClient.ts #295

Dayvvo opened this issue Jan 15, 2025 · 3 comments · May be fixed by #299

Comments

@Dayvvo
Copy link

Dayvvo commented Jan 15, 2025

Problem definition:

I noticed this issue when trying to connect wallets in my web app using bitcoin-connect. There were inconsistencies in the wait time when connecting a wallet as sometimes the wallet would load for a full minute which just seemed like bad UX for an end user. I traced this long wait time to the 1 minute replyTimeout found in the executeMultiNip47Request method when the getInfo method found in NWCClient.ts was called.

Fix

The 60 seconds timeout add an optional argument for timeout to the executeMultiNip47Request method. This allows getInfo to pass a shorter timeout duration when it is called. This would allow for a longer timeout when needed for other events like pay_invoice and a shorter timeout when it is needed
@Dayvvo
Copy link
Author

Dayvvo commented Jan 15, 2025

@rolznz let me know what you think!

@rolznz
Copy link
Contributor

rolznz commented Jan 16, 2025

@Dayvvo thanks for raising this! I agree, it would be nice to be able to pass custom values per call and have sensible defaults for different methods. e.g. payments could still be 60 seconds (sometimes they take longer than expected) but get_info could be 10 seconds.

@bumi @im-adithya what do you think?

@Dayvvo
Copy link
Author

Dayvvo commented Jan 17, 2025 •
edited
Loading

Alright! I'll open a PR regarding this. I'm thinking we could make the timeout customiseable for both replytimeout and publish timeout should there be a use case for it!

@Dayvvo Dayvvo linked a pull request Jan 20, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: add custom-timeout-values-for-requester-method Dayvvo/js-sdk
2 participants
@rolznz @Dayvvo