Skip to content

feat: add esplora client timeout option - #1119

Open
Arowolokehinde wants to merge 1 commit into
bitcoindevkit:masterfrom
Arowolokehinde:feat-esplora-client-timeout
Open

Arowolokehinde wants to merge 1 commit into
bitcoindevkit:masterfrom
Arowolokehinde:feat-esplora-client-timeout

Conversation

@Arowolokehinde

Copy link
Copy Markdown

Description

Closes #1109

EsploraClient::new had no way to set a request timeout: proxy was the
only setting it applied to esplora_client::Builder, leaving the
builder's timeout at None. A server that accepts a connection then stops
responding could block the calling thread indefinitely.

This adds an optional timeout (seconds), mirroring ElectrumClient::new. It
defaults to None, so callers opt in.

Notes to the reviewers

  • Defaults to None as suggested, so nothing changes for existing callers.
    Added a docstring note for the hang risk when it's unset.
  • Option<u8> seconds matches ElectrumClient.
  • Verified against a local stub server that accepts a connection and never
    replies. With timeout unset the call never returned; with timeout = 3 it
    raised Minreq("the timeout of the request was reached") after 3.01s —
    catchable rather than a hang.
  • No test added - this needs a server that accepts and then stalls.
  • I don't have permissions to add labels - could someone apply
    changelog: added?

Documentation

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing
  • I've added exactly one changelog:* label
  • I've linked the relevant upstream docs or specs above

New Features:

  • I've added docs for the new feature

EsploraClient::new had no way to set a request timeout: it only
forwarded proxy to esplora_client::Builder, leaving the builder's
timeout at None. A server that accepts a connection then stops
responding could block the calling thread indefinitely.

Add an optional timeout, as Option<u8> seconds to match
ElectrumClient::new. It defaults to None so callers opt in and existing
behavior is unchanged, and the docs note that a request can block
indefinitely when it is unset.

@j-kon j-kon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ACK 853a5ba

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Add a configurable request timeout to EsploraClient

2 participants