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

feat(iroh-bytes): add more context to errors #2196

Merged
merged 1 commit into from
Apr 16, 2024
Merged

Conversation

rklaehn
Copy link
Contributor

@rklaehn rklaehn commented Apr 16, 2024

Description

feat(iroh-bytes): add more context to errors

add hash and offset/range as context for LeafHashMismatch and ParentHashMismatch errors

This is what the log looks like now:

2024-04-16T07:17:19.613913Z  WARN iroh_bytes::provider: error: Error {
    context: "hash 17c8e5a70f69d289105553dfa762064e8cc57ffb9355d76508f557e37ef051db offset 9994240",
    source: LeafHashMismatch(
        ChunkNum(0x2620),
    ),
}
2024-04-16T07:17:26.664680Z  INFO magicsock{me=34qdsunbluss4zny}:actor:disco{node=kqn3pxyufuskwsup}: iroh_net::magicsock::node_map::best_addr: clearing best_addr reason=PongTimeout has_relay=true old_addr=192.168.1.131:64604
2024-04-16T07:18:05.507191Z  INFO poll_recv{me=34qdsunbluss4zny}:disco_in{node=kkpbi4eb5h6jcdjz src=Udp(192.168.1.131:62647)}: iroh_net::magicsock::node_map: inserting new node endpoint in NodeMap node=kkpbi4eb5h6jcdjz relay_url=None
2024-04-16T07:18:05.507311Z  INFO poll_recv{me=34qdsunbluss4zny}:disco_in{node=kkpbi4eb5h6jcdjz src=Udp(192.168.1.131:62647)}: iroh_net::magicsock::node_map::endpoint: new direct addr for node addr=192.168.1.131:62647
2024-04-16T07:18:05.510362Z  INFO poll_recv{me=34qdsunbluss4zny}:disco_in{node=kkpbi4eb5h6jcdjz src=Udp(192.168.1.131:62647)}: iroh_net::magicsock::node_map::best_addr: selecting new direct path for endpoint addr=192.168.1.131:62647 latency=2.686417ms trust_for=6.499932542s
2024-04-16T07:18:07.994402Z  INFO iroh_bytes::provider: sent 100390814 bytes in 2.4763055s
2024-04-16T07:18:07.994461Z  WARN iroh_bytes::provider: error: Error {
    context: "hash 370e2b3002be3b38b120f7b3be53da4cf646810e26f8f4a5018247c8188af5b2 range 65536..98304",
    source: Error {
        context: "g4hcwmacxy5trmja66z34u62jt3enaioe34pjjibqjd4qgek6wza",
        source: ParentHashMismatch(
            TreeNode::Branch(79, level=4),
        ),
    },
}

Notes & open questions

Change checklist

  • Self-review.
  • Documentation updates if relevant.
  • Tests if relevant.

add hash and offset/range as context for LeafHashMismatch and ParentHashMismatch errors
@rklaehn rklaehn requested a review from Frando April 16, 2024 07:22
@rklaehn rklaehn marked this pull request as ready for review April 16, 2024 07:23
@rklaehn
Copy link
Contributor Author

rklaehn commented Apr 16, 2024

@ppodolsky maybe this helps tracking down the weird errors.

@rklaehn rklaehn added this pull request to the merge queue Apr 16, 2024
Copy link
Contributor

@flub flub left a comment

Choose a reason for hiding this comment

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

I'd have done this using more tracing features, hence the notes I left. But this is fine as well.


Ok((SentStatus::Sent, size, file_reader.stats()))
}
_ => {
debug!("blob not found {}", hex::encode(name));
debug!("blob not found {}", hash.to_hex());
Copy link
Contributor

Choose a reason for hiding this comment

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

Usually it's better to do these as tracing fields:

Suggested change
debug!("blob not found {}", hash.to_hex());
debug!(hash = %hash.to_hex(), "blob not found");

Ok((SentStatus::NotFound, 0, SliceReaderStats::default()))
}
}
}

fn encode_error_to_anyhow(err: EncodeError, hash: &Hash) -> anyhow::Error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Because anyhow makes it particularly hard to downcast the hash (and rightly so, you probably need thiserror if that's what you want to support) another approach - which I usually take - is to add a tracing span with the hash because it's purely a reporting thing now. You can do an #[instrument(...)] to send_blob but probably also want to start the span even before this.

Merged via the queue into main with commit d3fec78 Apr 16, 2024
20 checks passed
ppodolsky pushed a commit to izihawa/iroh that referenced this pull request Apr 20, 2024
## Description

feat(iroh-bytes): add more context to errors

add hash and offset/range as context for LeafHashMismatch and
ParentHashMismatch errors

This is what the log looks like now:

```
2024-04-16T07:17:19.613913Z  WARN iroh_bytes::provider: error: Error {
    context: "hash 17c8e5a70f69d289105553dfa762064e8cc57ffb9355d76508f557e37ef051db offset 9994240",
    source: LeafHashMismatch(
        ChunkNum(0x2620),
    ),
}
2024-04-16T07:17:26.664680Z  INFO magicsock{me=34qdsunbluss4zny}:actor:disco{node=kqn3pxyufuskwsup}: iroh_net::magicsock::node_map::best_addr: clearing best_addr reason=PongTimeout has_relay=true old_addr=192.168.1.131:64604
2024-04-16T07:18:05.507191Z  INFO poll_recv{me=34qdsunbluss4zny}:disco_in{node=kkpbi4eb5h6jcdjz src=Udp(192.168.1.131:62647)}: iroh_net::magicsock::node_map: inserting new node endpoint in NodeMap node=kkpbi4eb5h6jcdjz relay_url=None
2024-04-16T07:18:05.507311Z  INFO poll_recv{me=34qdsunbluss4zny}:disco_in{node=kkpbi4eb5h6jcdjz src=Udp(192.168.1.131:62647)}: iroh_net::magicsock::node_map::endpoint: new direct addr for node addr=192.168.1.131:62647
2024-04-16T07:18:05.510362Z  INFO poll_recv{me=34qdsunbluss4zny}:disco_in{node=kkpbi4eb5h6jcdjz src=Udp(192.168.1.131:62647)}: iroh_net::magicsock::node_map::best_addr: selecting new direct path for endpoint addr=192.168.1.131:62647 latency=2.686417ms trust_for=6.499932542s
2024-04-16T07:18:07.994402Z  INFO iroh_bytes::provider: sent 100390814 bytes in 2.4763055s
2024-04-16T07:18:07.994461Z  WARN iroh_bytes::provider: error: Error {
    context: "hash 370e2b3002be3b38b120f7b3be53da4cf646810e26f8f4a5018247c8188af5b2 range 65536..98304",
    source: Error {
        context: "g4hcwmacxy5trmja66z34u62jt3enaioe34pjjibqjd4qgek6wza",
        source: ParentHashMismatch(
            TreeNode::Branch(79, level=4),
        ),
    },
}
```

<!-- A summary of what this pull request achieves and a rough list of
changes. -->

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

## Change checklist

- [ ] Self-review.
- [ ] Documentation updates if relevant.
- [ ] Tests if relevant.
@rklaehn rklaehn deleted the better-send-logging branch November 20, 2024 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants