Skip to content

Fetching ohttp-keys should time out quickly#1005

Merged
DanGould merged 1 commit into
payjoin:masterfrom
zealsham:fetch-ohttp-timeout
Aug 28, 2025
Merged

Fetching ohttp-keys should time out quickly#1005
DanGould merged 1 commit into
payjoin:masterfrom
zealsham:fetch-ohttp-timeout

Conversation

@zealsham

@zealsham zealsham commented Aug 27, 2025

Copy link
Copy Markdown
Collaborator

This pr adds a timeout time of 10 seconds when trying to connect to a relay and the response time is longer than normal . addresses #968

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.

This pr adds a timeout time of 10 seconds when trying
to connect to a relay and for some reason it fails .
addresses payjoin#968
@coveralls

Copy link
Copy Markdown
Collaborator

Pull Request Test Coverage Report for Build 17278728526

Details

  • 6 of 12 (50.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.04%) to 85.676%

Changes Missing Coverage Covered Lines Changed/Added Lines %
payjoin/src/core/io.rs 6 12 50.0%
Totals Coverage Status
Change from base Build 17278137158: -0.04%
Covered Lines: 7973
Relevant Lines: 9306

💛 - Coveralls

@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.

Primo. Nice and easy.

utACK 034e89d

Have you smoke tested this at all to iterate through bad relays? I wonder how the experience is vs 5 seconds.

@DanGould

Copy link
Copy Markdown
Member

This has me wondering also, since fetching can happen multiple times, if the client should later be cached or re-used so that it can get a performance edge or if that only makes sense for custom clients and not the one we ship with io

@spacebear21

Copy link
Copy Markdown
Collaborator

Tangential to this issue, I also wonder if implementers could parallelize requests to multiple ohttp relays instead of trying them sequentially, and process whatever response returns first.

@DanGould

Copy link
Copy Markdown
Member

It'd be nice to be able to test this experience, ESPECIALLY IN MOBILE! but I'll merge this for now. I'll open an "audit network disruption experience" issue.

@DanGould
DanGould merged commit 21e6b9a into payjoin:master Aug 28, 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.

4 participants