Skip to content

[app] Audit let _ = sites in network_bridge.rs for swallowed errors #124

@intendednull

Description

@intendednull

Parent: #108

Problem

crates/app/src/network_bridge.rs is peppered with let _ = event_tx.send(...) calls. Most of these are benign — event_tx is std::sync::mpsc::Sender<NetworkBridgeEvent>, which is unbounded, so send() only fails when the receiver is dropped during shutdown.

But not all of them are benign. Line 347:

NetworkEvent::ChunkRequested { channel, hash, .. } => {
    let response = if let Some(data) = file_mgr.get_chunk(&hash) {
        ChunkResponse::Found { hash, data: data.to_vec() }
    } else {
        ChunkResponse::NotFound { hash }
    };
    let _ = node.respond_chunk(channel, response);
}

node.respond_chunk returns a real error — if it fails, the requester hangs waiting for a chunk that will never arrive. There are a handful of other non-mpsc let _ = sites that similarly swallow real errors.

Fix

  1. Grep crates/app/src/network_bridge.rs for all let _ = occurrences.
  2. Classify each:
    • event_tx.send(...) — unbounded mpsc, safe to keep silent (or add a shutdown-only if let Err(...) with a debug! log).
    • Any other Result-returning call (respond_chunk, identity saves, etc.) — log at warn level with context and, where possible, mark the associated state so the UI can surface it.
  3. For respond_chunk specifically, at least a warn!(%hash, %err, "failed to respond to chunk request") line.

Out of scope

This issue is an audit, not a refactor. Don't change the mpsc channel type. That's a separate larger change if we ever decide we want backpressure.

Test

For respond_chunk, add a unit test that forces the channel to fail and asserts a warning is logged.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions