Add `InProgress` variant to `io::ErrorKind`
Proposal
Add InProgress variant to io::ErrorKind to support cases where OS error is libc::EINPROGRESS
Problem statement
While working on API that relies on io::last_os_error I found out that kind field of io::Error is set to Uncategorized which means I can't "safely" use .kind() to check if it's a specific error, in our case libc::EINPROGRESS
Motivation, use-cases
Our code example that can be simplified given the addition.
Solution sketches
Links and related work
Sent a PR of the proposed change https://github.com/rust-lang/rust/pull/101155#pullrequestreview-1088931007
@thomcc brought up in the PR that it's similar to WouldBlock. I do agree but my objective is to be aware when EINPROGRESS happened and that's Linux error codes quirks. To be honest I'm okay with matching EINPROGRESS to WouldBlock but it might be weird and intuitive for most people.
I don't think it makes sense for us to reflect all errno values in ErrorKind -- this is what raw_os_error is for, after all.
But, on the one hand, WouldBlock is quite specific -- it definitely implies that it maps to EWOULDBLOCK.
I don't think it makes sense for us to reflect all errno values in
ErrorKind-- this is whatraw_os_erroris for, after all.But, on the one hand,
WouldBlockis quite specific -- it definitely implies that it maps toEWOULDBLOCK.
I'm fine with that. I am not really synced with the general thought of best practice in this so really up to the maintainers 😊
Took @thomcc and changed the PR to use WouldBlock error instead of introducing new variant, InProgress
Sounds like this got resolved without adding this variant.
We may, in the future, want to add such a variant for other purposes, but it sounds like we can close this ACP for now.
This seems to have gotten closed on the theory that EINPROGRESS could get mapped to WouldBlock, but as discussed on the PR and in today's libs-api meeting, that's not the right mapping. If we do provide a mapping for this, it should be to a separate ErrorKind variant.
We don't have a consensus in @rust-lang/libs-api to create an ErrorKind variant for this.
Leaving this open in the hopes that we reach a consensus to do so.
Thanks for discussing this.
I believe that EINPROGRESS is quite commonly used - it isn't a seldom error so if there's anything I can do to convince adding it - I'd love to push for it :)
Originally we thought it'd make sense to merge this with ErrorKind::WouldBlock, but then later it seemed clear that EAGAIN and EINPROGRESS should not be treated the same in most cases.
However, when we try to discuss this in the team meeting, we get stuck on not having a complete overview of how these errors occur and are handled in practice, especially on different platforms.
So, to move this forward, we'll need a bit more context to answer questions like: When does this error occur? How does one usually handle this error? Does it exist on non-unix platforms? If so, does it represent the exact same thing? Would adding this ErrorKind result in more or less platform-specific code in practice?
Thanks @m-ou-se - I will try to provide my findings.
Personally, I don't think that mapping EINPROGRESS to WouldBlock would make sense tbh (and that was my opinion since I created thie issue)
When does it occur?
It occurs when you call a procedure which can take time to complete, and isn't blocking to completion - in sockets for example when the socket is set to not block, and you call connect it'd return immediately with the EINPRGORESS.
Example from stdlib: https://github.com/rust-lang/rust/blob/c1699a79a603faa8b522a4b51553b6781913a50e/library/std/src/sys/solid/net.rs#L242
How does one usually handle this error?
It's considered a non-error, it's a way of letting the caller know that the operation didn't fail but also didn't complete yet. We originally faced it because our code "errored" while getting this while it's not a "true" error. The handling is success - continue.
Does it exist on non-unix platforms?
It exists on Windows and other POSIX-compliant systems, not sure about other?
The representation of EINPRGORESS across platforms is same, and in all isn't same as EWOULDBLOCK or EAGAIN.
Also seems all (?) platforms on rust's libc implement it.
Would adding this ErrorKind result in more or less platform-specific code in practice?
Hard to say, but right now we're just accessing the raw os instead of using the variant, which what annoyed me to create this issue.