magic-wormhole / magic-wormhole.rs

Rust implementation of Magic Wormhole, with new features and enhancements
European Union Public License 1.2
648 stars 72 forks source link

transfer: receive wormhole close should not throw error when failed #159

Closed felinira closed 2 years ago

felinira commented 2 years ago

When file transfer is complete, the receiving end sends a wormhole.close() event when the transfer completed successfully. If this message fails the transfer will return a WormholeError.

Considering the fact that transfers can take very long, and the rendezvous server might be down or being restarted (which would be enough to trigger the error) while the transfer is occurring, this should probably not fail or at least return a very distinguishable error message that indicates the fact the transfer itself actually completed successfully.

piegamesde commented 2 years ago

So basically are you suggesting to change self.wormhole.close().await?; to let _ = self.wormhole.close().await;, like done elsewhere?

felinira commented 2 years ago

I think this wouldn't be too bad. At least i have no use for additional error reporting in that case. The file got transferred successfully which IMHO is the most important information, so I would be fine with just throwing the error away.

piegamesde commented 2 years ago

This should be feasible. We should check other similar code paths for error handling in order to have consistent behavior.

felinira commented 2 years ago

I can confirm that this works for me now. Thanks :)

meejah commented 2 years ago

or being restarted (which would be enough to trigger the error

Might be worth noting that the mailbox server doesn't lose state when re-started.

(For the existing transfer protocol I don't think this really matters, since even if you re-establish the mailbox connection there's no way to re-do the transfer .. but thought it worth clarifying that point :) )