netbase: clean up Proxy logging #29882

pull pinheadmz wants to merge 1 commits into bitcoin:master from pinheadmz:cleaner-proxy-logs changing 1 files +1 −5
  1. pinheadmz commented at 6:22 pm on April 15, 2024: member

    Follow up to #27375 and see #29649 (comment)

    This removes an extra log message when we can’t connect to our own proxy, and another when the proxy is invalid.

    Before #27375 if proxy is unreachable

    02024-04-15T17:54:51Z connect() to 127.0.0.1:9999 failed after wait: Connection refused (61)
12024-04-15T17:54:52Z connect() to 127.0.0.1:9999 failed after wait: Connection refused (61)
22024-04-15T17:54:52Z connect() to 127.0.0.1:9999 failed after wait: Connection refused (61)
32024-04-15T17:54:53Z connect() to 127.0.0.1:9999 failed after wait: Connection refused (61)
42024-04-15T17:54:53Z connect() to 127.0.0.1:9999 failed after wait: Connection refused (61)

    After #27375 if unix proxy is unreachable:

    02024-04-15T17:54:03Z connect() to /Users/matthewzipkin/Desktop/tor failed: No such file or directory (2)
12024-04-15T17:54:03Z Cannot connect to socket for /Users/matthewzipkin/Desktop/tor
22024-04-15T17:54:04Z connect() to /Users/matthewzipkin/Desktop/tor failed: No such file or directory (2)
32024-04-15T17:54:04Z Cannot connect to socket for /Users/matthewzipkin/Desktop/tor
42024-04-15T17:54:04Z connect() to /Users/matthewzipkin/Desktop/tor failed: No such file or directory (2)
52024-04-15T17:54:04Z Cannot connect to socket for /Users/matthewzipkin/Desktop/tor
62024-04-15T17:54:05Z connect() to /Users/matthewzipkin/Desktop/tor failed: No such file or directory (2)
72024-04-15T17:54:05Z Cannot connect to socket for /Users/matthewzipkin/Desktop/tor

    After this PR:

    02024-04-15T18:18:51Z connect() to /Users/matthewzipkin/Desktop/tor failed: No such file or directory (2)
12024-04-15T18:18:51Z connect() to /Users/matthewzipkin/Desktop/tor failed: No such file or directory (2)
22024-04-15T18:18:52Z connect() to /Users/matthewzipkin/Desktop/tor failed: No such file or directory (2)
32024-04-15T18:18:52Z connect() to /Users/matthewzipkin/Desktop/tor failed: No such file or directory (2)
  2. DrahtBot commented at 6:22 pm on April 15, 2024: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK tdb3, laanwj

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #29641 (scripted-diff: Use LogInfo/LogDebug over LogPrintf/LogPrint by maflcko)

    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.

  3. in src/netbase.cpp:635 in d1af4422d1 outdated 
    631@@ -632,11 +632,6 @@ std::unique_ptr<Sock> ConnectDirectly(const CService& dest, bool manual_connecti
632 
633 std::unique_ptr<Sock> Proxy::Connect() const
634 {
635-    if (!IsValid()) {
    laanwj commented at 8:06 pm on April 15, 2024:
    It’s probably a good idea to keep the !IsValid check here, just to be sure. Fine with removing the log.
    tdb3 commented at 3:00 am on April 16, 2024:

    I could be mistaken, but it looks at first glance that the same result return {} would be produced in ConnectToSocket(). I don’t think it hurts anything to keep the check though (does seem safer, or at least more readable).

    https://github.com/bitcoin/bitcoin/blob/d1af4422d13ba84253a1555c2fe5a42170fa8b35/src/netbase.cpp#L603-L604

    and reached in Proxy::Connect() through either the call to ConnectToSocket(): https://github.com/bitcoin/bitcoin/blob/d1af4422d13ba84253a1555c2fe5a42170fa8b35/src/netbase.cpp#L653-L655

    or by way of ConnectDirectly(): https://github.com/bitcoin/bitcoin/blob/d1af4422d13ba84253a1555c2fe5a42170fa8b35/src/netbase.cpp#L626-L628

    laanwj commented at 5:00 am on April 16, 2024:

    Yes, it’s not strictly currently necessary, but code changes sometimes. It’s good to have error and unexpected situation checking in place, instead of crashing later on.

    Edit: also removing a check is not strictly part of “cleaning up logging”.

    pinheadmz commented at 2:22 pm on April 16, 2024:
    Restoring the check, without the log and revising the PR description.
  4. tdb3 commented at 3:30 am on April 16, 2024: contributor

    ACK for d1af4422d13ba84253a1555c2fe5a42170fa8b35 Good cleanup, making logs less cluttered while keeping relevant connection state logging.

    In addition to observing the lack of Cannot connect to socket for message, also ran a quick test where Tor was stopped then started again. bitcoind was still able to detect and inform the user of the loss of the proxy and subsequent peer connected message provided indication that connection through proxy was reestablished.

     013308 tx=3456746 date='2024-04-16T03:14:24Z' progress=0.999999 cache=0.3MiB(285txo)
 12024-04-16T03:23:15Z UpdateTip: new best=0000009be064b2b97227938e8e391444d13f6a3495ae62eb3233aa93d52abee5 height=191494 version=0x20000000 log2_work=41.113316 tx=3456773 date='2024-04-16T03:17:56Z' progress=0.999999 cache=0.3MiB(358txo)
 22024-04-16T03:23:15Z New outbound-full-relay v1 peer connected: version: 70016, blocks=191494, peer=2
 32024-04-16T03:23:16Z New outbound-full-relay v1 peer connected: version: 70016, blocks=191494, peer=3
 42024-04-16T03:23:18Z P2P peers available. Skipped DNS seeding.
 52024-04-16T03:23:18Z dnsseed thread exit
 6[killed Tor]
 72024-04-16T03:23:21Z [error] Error while reading proxy response
 82024-04-16T03:23:22Z connect() to /tmp/torsock failed: No such file or directory (2)
 92024-04-16T03:23:22Z connect() to /tmp/torsock failed: No such file or directory (2)
102024-04-16T03:23:23Z connect() to /tmp/torsock failed: No such file or directory (2)
112024-04-16T03:23:23Z connect() to /tmp/torsock failed: No such file or directory (2)
122024-04-16T03:23:24Z connect() to /tmp/torsock failed: No such file or directory (2)
132024-04-16T03:23:24Z connect() to /tmp/torsock failed: No such file or directory (2)
14[Started Tor again]
152024-04-16T03:23:29Z New outbound-full-relay v1 peer connected: version: 70016, blocks=191494, peer=4
  5. laanwj added the label P2P on Apr 16, 2024
  6. laanwj added the label Utils/log/libs on Apr 16, 2024
  7. netbase: clean up Proxy logging fb4cc5f423
  8. pinheadmz force-pushed on Apr 16, 2024
  9. tdb3 commented at 4:03 pm on April 16, 2024: contributor
    CR ACK for fb4cc5f423ce587c1e97377e8afdf92fb4850f59
  10. laanwj approved
  11. laanwj commented at 8:15 am on April 24, 2024: member
    ACK fb4cc5f423ce587c1e97377e8afdf92fb4850f59
  12. fanquake merged this on Apr 24, 2024
  13. fanquake closed this on Apr 24, 2024
  14. dasibcryptoidology approved


pinheadmz DrahtBot laanwj tdb3 dasibcryptoidology

Labels
P2P Utils/log/libs

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-21 09:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me