Skip to content

getpeerinfo (and ping/getconnectioncount) log dropped responders as ERROR, dumping the full response payload + build path #1181

Description

@jvsena42

Summary

perform_user_request wraps several oneshot::Sender::send calls in try_and_log!. When the receiver has already been dropped (the RPC caller timed out or moved on before the node replied), oneshot::send returns Err(value) — where value is the response you tried to send. try_and_log! then logs that value at ERROR level, so a benign dropped-receiver turns into a noisy, misleading error line that also dumps the response payload and leaks the builder's absolute source path.

Affected sites

crates/floresta-wire/src/p2p_wire/node/user_req.rs (on master):

  • L47 try_and_log!(responder.send(NodeResponse::GetPeerInfo(peers))); ← worst: payload is the whole peer list
  • L75 try_and_log!(responder.send(NodeResponse::Ping(true)));
  • L95 try_and_log!(responder.send(NodeResponse::GetConnectionCount(count)));
  • L166 try_and_log!(responder.send(NodeResponse::GetAddrManInfo(info)));

try_and_log! (crates/floresta-common/src/macros.rs:162) expands to:

tracing::error!("{}: {} - {:?}", line!(), file!(), error);

Observed impact

Running a node behind a client that polls getpeerinfo regularly, every poll whose receiver is dropped produces one line like:

ERROR floresta_wire::p2p_wire::node::user_req: 47: /home/<user>/.../crates/floresta-wire/src/p2p_wire/node/user_req.rs - GetPeerInfo([PeerInfo { id: 27, address: 38.242.165.208:8333, services: ServiceFlags(3149), user_agent: "/Satoshi:31.0.0/", ... }, ... ])

On one real device this was 296 of 320 total ERROR lines in the log. Three distinct problems:

  1. Not an error — a dropped responder is expected and harmless, but it's logged at ERROR.
  2. Payload dump — the failed send returns the response, so {:?} prints the entire peer list (or addrman info) on one line.
  3. Path leakfile!() resolves to the build machine's absolute checkout path.

Suggested fix

A dropped responder in these handlers is benign, so discard the result with let _ = responder.send(...), matching the other responder.send sites in the same file that already do this. (If you want visibility, debug! would be appropriate — but not ERROR, and never the payload.)

More broadly, try_and_log! on a oneshot::Sender::send is an anti-pattern: because the Err carries the un-received value, any dropped receiver re-logs the full payload plus the source path. Might be worth auditing other try_and_log!(…send(…)) uses.

Metadata

Metadata

Assignees

No one assigned

    Labels

    loggingImprovements to log messages, structure, verbosity, formatting and diagnostics

    Type

    Fields

    No fields configured for Bug.

    Projects

    Status
    Backlog

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions