fix(iroh-blobs): Remove debugging logs & more cleanup #2690
Conversation
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/2690/docs/iroh/ Last updated: 2024-09-02T16:01:23Z |
|
I originally planned to see if this helps surface some issues Pasha was seeing & if so, perhaps add more updates to this branch. |
| Err(err) => { | ||
| // This prints a "FailureAction" which is somewhat weird, but that's all we get here. | ||
| tracing::error!(?err, "failed queuing new download"); |
There was a problem hiding this comment.
we already return an error DownloadError::DownloadFailed which admittedly does not say a lot, but the error case is handled, so I honestly do not see what do we gain from this addition
There was a problem hiding this comment.
The idea is to log the error cause.
| @@ -717,7 +717,6 @@ impl<G: Getter<Connection = D::Connection>, D: Dialer> Service<G, D> { | |||
| entry.get_mut().intents.insert(intent_id, intent_handlers); | |||
| } | |||
| hash_map::Entry::Vacant(entry) => { | |||
| tracing::warn!("is new, queue"); | |||
There was a problem hiding this comment.
uhhh this is(was) ugly
|
@divagant-martian did you intentionally enable auto-merge? You didn't approve the PR 😅 |
|
I did. I also wanted to know what were you looking for by logging that error. It's still better than before so let's go ahead |
There's a thread in discord that has some details. More specifically, any errors in I hope I remember the part of the code correctly - following the whole call structure took some time. But as far as I remember failures end up as this |
Description
More stuff to clean up I came across while trying to debug an issue.
Also, this makes it so that errors from
iroh_blobs::get::db::get_blobthat happen before theget_connstep get printed.Breaking Changes
None.
Notes & open questions
I decided to just remove the tracing logs in
downloader/progress.rs. They never really helped us surface the bug, they mostly polluted the logs, to be honest.The only thing they surfaced was that we didn't properly clean up
TransferState::progress_id_to_blobwhen there's an error iniroh_blobs::get::db::get_blobthat causes us to not send theDownloadProgress::Doneevent.Change checklist
[ ] Documentation updates following the style guide, if relevant.[ ] Tests if relevant.