refactor(iroh): log inner errors #2423
Conversation
we currently abort the loop on outer errors (errors from spawn). But we do not look at inner errors (task returns properly but returns an error) at all. This logs the inner errors as debug! and coninues.
Don't exit the main loop just because an inner task is cancelled.
| Some(Err(outer)) => { | ||
| if outer.is_panic() { | ||
| error!("Task panicked: {outer:?}"); | ||
| break; | ||
| } else if outer.is_cancelled() { | ||
| debug!("Task cancelled: {outer:?}"); | ||
| } else { | ||
| error!("Task failed: {outer:?}"); | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
Do we need to print cancellations?
And IIUC, the third case is impossible. It's either a cancel or a panic: https://docs.rs/tokio/latest/src/tokio/runtime/task/error.rs.html#15-18
| Some(Err(outer)) => { | |
| if outer.is_panic() { | |
| error!("Task panicked: {outer:?}"); | |
| break; | |
| } else if outer.is_cancelled() { | |
| debug!("Task cancelled: {outer:?}"); | |
| } else { | |
| error!("Task failed: {outer:?}"); | |
| break; | |
| } | |
| } | |
| Some(Err(outer)) if outer.is_panic() => { | |
| error!("Task panicked: {outer:?}"); | |
| break; | |
| } |
There was a problem hiding this comment.
Do we need to print cancellations? And IIUC, the third case is impossible. It's either a cancel or a panic: https://docs.rs/tokio/latest/src/tokio/runtime/task/error.rs.html#15-18
Yeah, I thought so as well, but then why don't they expose an enum? The way the JoinError API is written it seems that they want to keep the possibility open to have a third case.
There was a problem hiding this comment.
Do we need to print cancellations?
Not sure if they would ever happen at all. But if it happens, I would like to know.
|
What do you think of renaming this PR |
Not sure. It does not change the behaviour of the program except for logging, so calling it a fix seems wrong. |
Description
we currently abort the loop on outer errors (errors from spawn). But we do not look at inner errors (task returns properly but returns an error) at all.
This logs the inner errors as debug! and coninues.
Also checks if the outer error is actually a panic, and continues the loop otherwise.
Breaking Changes
Notes & open questions
Change checklist