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

rpcclient: add timeout to http request. #2256

Merged
merged 1 commit into from
Oct 11, 2024

Conversation

ziggie1984
Copy link
Contributor

@ziggie1984 ziggie1984 commented Sep 30, 2024

Not sure why we did not have a timeout in the first place, but I think it makes sense. Did not choose to add it as a config value but can be changed.

@coveralls
Copy link

coveralls commented Sep 30, 2024

Pull Request Test Coverage Report for Build 11269939212

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 45 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.08%) to 57.149%

Files with Coverage Reduction New Missed Lines %
peer/peer.go 9 73.49%
wire/msgaddrv2.go 16 51.72%
wire/netaddressv2.go 20 74.45%
Totals Coverage Status
Change from base Build 11236152006: -0.08%
Covered Lines: 29816
Relevant Lines: 52172

💛 - Coveralls

@ziggie1984 ziggie1984 marked this pull request as ready for review October 2, 2024 22:46

// defaultHTTPTimeout is the default timeout for an http request, so the
// request does not block indefinitely.
defaultHTTPTimeout = time.Second * 10
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is usually 60s in "traditional" business...

@ziggie1984 ziggie1984 force-pushed the timeout-https-client branch 2 times, most recently from 0a8cb9c to 19f48ae Compare October 10, 2024 08:03
@guggero guggero merged commit 24eb815 into btcsuite:master Oct 11, 2024
3 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.

6 participants