update deps #2107
update deps #2107
Conversation
|
There are warnings about unused results, more |
dignifiedquire
force-pushed
the
update-deps
branch
2 times, most recently
from
2d00d3e to
f8ac0c2
Compare
| @@ -321,7 +321,7 @@ async fn start(args: Vec<String>) -> Result<(), Error> { | |||
| Ok(line) => { | |||
| // TODO: ignore "set mail_pw" | |||
| rl.add_history_entry(line.as_str()); | |||
| async_std::task::block_on(reader_s.send(line)); | |||
| async_std::task::block_on(reader_s.send(line)).unwrap(); | |||
There was a problem hiding this comment.
This SendError shouldn't happen because the channel should never be closed, but I'd rather print the error instead of panic here.
There was a problem hiding this comment.
It shouldn't happen, and it is just the repl, which is why an unwrap seems appropriate to me here
| started | ||
| .send(()) | ||
| .await | ||
| .expect("inbox loop, missing started receiver"); |
There was a problem hiding this comment.
Same for all expect calls, can they be replaced with warnings written into logs? This way if someone accidentally kills the other side of the channel, we get an app that does not do networking, but still does not crash and allows to collect the log with these errors.
There was a problem hiding this comment.
I am worried that if we don't panic, the app will be in a state in which it is unclear what is happening to the user, as it might seem to be working, except no incoming or outgoing messages are happening. So I would rather crash the app, ensuring it gets restarted and we get a crash report hopefully
There was a problem hiding this comment.
Crash report will be hard to debug in this case. If nothing is sent/received, user will be able to send us a log, which is much more useful. Maybe throw an error!?
There was a problem hiding this comment.
This is actually a pretty serious logic error that would not pass the integration test suite. It's sad that this can't be done using type-safety, but I agree that panicking here is best as otherwise it would be harder to detect in early testing.
| started | ||
| .send(()) | ||
| .await | ||
| .expect("inbox loop, missing started receiver"); |
There was a problem hiding this comment.
This is actually a pretty serious logic error that would not pass the integration test suite. It's sad that this can't be done using type-safety, but I agree that panicking here is best as otherwise it would be harder to detect in early testing.
|
I took the liberty to update this, stop-token doesn't support async-std 1.9 yet so had to limit it to 1.8 for now which still provides the old channel API. If @dignifiedquire could have another look that I didn't screw anything up I think we should merge it. |
|
I've created async-rs/stop-token#7 to allow us to move to async-std 1.9. Hopefully they'll merge and release something like that soon. |
and switch to new channels in [email protected]
The stop-token crate does net yet support a more recent async-std.
and switch to new channels in [email protected]