Add InProgress variant to io::ErrorKind

[aviramha]

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
rust-lang/rust#101155 (review)

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

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

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

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]

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

[joshtriplett]

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]

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]

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.

[aviramha]

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

[m-ou-se]

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?

[aviramha]

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.

[joshtriplett]

We discussed this once more in today's libs-api meeting, and confirmed that we want this to be a separate ErrorKind variant, InProgress.

The primary intended distinction here (various platform portability issues aside) is that WouldBlock means you should retry the operation when ready, and InProgress means you shouldn't retry the operation.

[aviramha]

Woohoo! Great news. Is it okay if I reopen my PR?

[joshtriplett]

@aviramha Please do reopen, or make a new one, whichever is simpler for providing a PR that applies.

[aviramha]

Done rust-lang/rust#130789