libs-team icon indicating copy to clipboard operation
libs-team copied to clipboard

Add `InProgress` variant to `io::ErrorKind`

Open aviramha opened this issue 3 years ago • 10 comments

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

aviramha avatar Aug 29 '22 16:08 aviramha

@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.

aviramha avatar Aug 29 '22 16:08 aviramha

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.

thomcc avatar Aug 29 '22 17:08 thomcc

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'm fine with that. I am not really synced with the general thought of best practice in this so really up to the maintainers 😊

aviramha avatar Aug 29 '22 17:08 aviramha

Took @thomcc and changed the PR to use WouldBlock error instead of introducing new variant, InProgress

aviramha avatar Sep 17 '22 12:09 aviramha

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.

joshtriplett avatar May 17 '23 13:05 joshtriplett

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.

joshtriplett avatar Aug 01 '23 15:08 joshtriplett

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.

joshtriplett avatar Aug 01 '23 15:08 joshtriplett

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 :)

aviramha avatar Aug 01 '23 17:08 aviramha

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?

m-ou-se avatar Aug 15 '23 13:08 m-ou-se

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.

aviramha avatar Aug 15 '23 14:08 aviramha