Part of the effort to remove the libevent dependency altogether, see #31194
This takes the HTTPHeaders class from #32061 and puts it into common/http.h|cpp. While it would not be strictly required for that PR to go in first, it might be better to review it first or simultaneously.
Otherwise the change itself replaces the libevent-based HTTP client with a simple synchronous implementation which uses the Sock class directly.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34342.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| Concept ACK | pinheadmz, bradleystachurski |
If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
🚧 At least one of the CI tasks failed.
Task tidy: https://github.com/bitcoin/bitcoin/actions/runs/21139253857/job/60789761512
LLM reason (✨ experimental): Clang-Tidy failure: modernize-use-starts-ends-with flags substr usage in bitcoin-cli.cpp, treated as error and causing CI to fail.
Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
This will be shared between the http server and the bitcoin-cli http client code.
Concept ACK (and supportive of the broader libevent removal effort)
Reviewed efd74a822d9577c53259b3d27526957b0674f7d8
Found a minor behavioral regression: connection failures (e.g. unresolvable host, non-listening port) lose the help text shown on master.
Master:
0error: timeout on transient error: Could not connect to the server 127.0.0.1:19999
1Make sure the bitcoind server is running and that you are connecting to the correct RPC port.
2Use "bitcoin-cli -help" for more info.
Branch:
0error: timeout on transient error: Could not connect to the server 127.0.0.1:19999
Catching instead of re-throwing preserves the help text:
0diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp
1index 8414ad0317..3cd6e4b29c 100644
2--- a/src/bitcoin-cli.cpp
3+++ b/src/bitcoin-cli.cpp
4@@ -1258,7 +1258,7 @@ static UniValue CallRPC(BaseRequestHandler* rh, const std::string& strMethod, co
5 try {
6 response = client.Post(endpoint, headers, strRequest);
7 } catch (const CConnectionFailed&) {
8- throw;
9+ response.status = 0;
10 }
11
12 if (response.status == 0) {
Verified this preserves the help text and passes interface_bitcoin_cli.py.
Replace libevent-based HTTP client with a simple synchronous implementation using the Sock class directly.
Found a minor behavioral regression: connection failures (e.g. unresolvable host, non-listening port) lose the help text shown on master.
You were right, good catch! Your suggested fix looks good to me as well, I have just applied that change.