Tracking Issue for io_error_more
Opened this issue ยท 93 comments
Feature gate: #![feature(io_error_more)]
This is a tracking issue for many io::ErrorKind variants being added in #79965.
The additions were motivated by a perusal of Unix standards, but corresponding mappings for Windows were included in that PR.
Public API
std::io::ErrorKind::Foo for many Foo. I will paste a list here when the MR is merged :-).
Steps / History
- Implementation: #79965
- Final commenting period (FCP)
- Partial Stabilization - #128316
- Stabilization PR
Unresolved Questions
- What is our general policy about
ErrorKindvariants ? It may not be necessary to resolve this wider question in order to stabilise this particular set ofErrorKinds.
What is our general policy about ErrorKind variants ?
Just a bit of user feedback, this PR landing in beta broke our CI as we were asserting certain io Errors. I dunno if you can really call it a "language-breaking" change, but it did seem surprising that our CI would fail on beta due to rust lib changes.
https://doc.rust-lang.org/std/path/struct.Path.html#method.is_dir documents:
This is a convenience function that coerces errors to
false. If you want to check errors, callfs::metadataand handle itsResult. Then callfs::Metadata::is_dirif it wasOk.
Iโve implemented this by coercing ErrorKind::NotFound to false and propagating other errors. However some of my tests fail in cases where ErrorKind::NotADirectory is (or would be) returned, showing that it should also be coerced to false.
This and the previous comment show that these variants are not like other unstable features that crates who donโt opt-in can pretend donโt exist, they can already be present in errors returned by the standard library. Iโd like @rust-lang/libs to consider stabilizing them soon.
@TheBlueMatt, was your code that broke expecting ErrorKind::Other or does this also affect values that previously were, well, other than Other?
@TheBlueMatt, was your code that broke expecting ErrorKind::Other or does this also affect values that previously were, well, other than Other?
Yes, it was checking that an error was Other. Still, that seems like a good enough reason that this can't be stabilized as-is? Its not unreasonably that some non-test code could be checking for Other for one reason or another, no?
@TheBlueMatt Other has been documented for some time as not something you should directly check for:
Errors that are Other now may move to a different or a new ErrorKind variant in the future. It is not recommended to match an error against Other and to expect any additional characteristics, e.g., a specific Error::raw_os_error return value.
#85746 is the change that made the standard library stop returning Other, which was a prerequisite for adding new ErrorKind values.
We do want to stabilize these new variants soon. But in the meantime, if you have code that matches Other and checks a specific underlying error code, you should be able to change that to match _ and check the specific error code.
@TheBlueMatt Other has been documented for some time as not something you should directly check for:
See-also discussion ending at #79965 (comment), but the specific documentation that isn't only lightly implying this being an issue was added middle of last year (which is much more recent than our test code here). Indeed, it was implied earlier, but for those of us who weren't thinking really deeply about the implications of the struct-level docs it didn't come across entirely.
you should be able to change that to match _ and check the specific error code.
Right, we'd have to do even more platform-specific stuff, which I'm a bit hesitant to do, I guess we can just drop our tests, but I do worry about other code that may exist that isn't pure-tests from other projects.
I'm honestly somewhat surprised this is moving forward without some attempts at a rustc warning better communicating to users that the code they have written that compiles and runs is actually undefined behavior (not in the C sense of may-eat-your-cat, but in the may-change-in-the-future, behavior is not guaranteed sense). I understand that may not be practical to hit in all cases, but I imagine at least some direct match .. Other or if == Other things could likely be detected. In general, I am very surprised this doesn't fall under rustc's stability guarantees around running, operational, functional code in the wild not being broken by future versions of rustc.
Hi, @TheBlueMatt
As the author of one of the MRs which actually broke your test, I would like to say that I'm sorry about that. Unfortunately, risking breakage like yours seemed like the best option. I know it sucks when you're on the wrong side of a tradeoff like this - especially when it suddenly breaks your stuff.
You are right to say that it would have been better to have arranged to warn about this. But doing so in rustc is not currently straightforward. ErrorKind::Other had to be a stable variant, because users of the stdlib need to be able to construct errors with that kind. But we do still need to be able to add variants, rather than having this enum frozen for all time.
Perhaps if we had thought of it sooner, a clippy lint would have made sense. But we didn't have one. (And maybe that wouldn't have helped you; not everyone runs clippy. I know I generally don't, despite feeling guilty about that...)
But given where we were, our sensible options were risking this kind of breakage, or postponing the addition of new ErrorKinds (perhaps for some considerable time) while we did more preparation (eg trying to warn people).
My view is that the new ErrorKinds are very sorely needed. For example, just this week I was writing a program that needs to read configuration from filesystem objects which might be files, or directories. The IsADirectory error from File::open makes that more convenient to do portably and avoids having to ever discard an ErrorKind::Other on the basis of complex reasoning about the filesystem state.
Whether avoiding a substantial delay to new ErrorKinds was worth risking this kind of breakage seems like something reasonable people can disagree on. As others have said, one justification is that the documentation has always said that ErrorKind might grow new variants, and to my mind the implication that those new variants would probably appear instead of Other is obvious. I guess that is cold comfort to you.
Another mitigation is that even when this causes breakage, it is likely to be in tests (as in your case) - ie, the risk of bad code being deployed as a result is considerably lower than that of a test failure.
Having said all that: given this is only in beta, I think we do have the option to back this out ? If we did that, how long would we wait ? Who would write the missing clippy lint or whatever ?
Personally I'm torn. Rust's care for stability is one of its great attractions to me and I regret having undermined that for some of Rust's users. On the other had the previous ErrorKind enum was IMO seriously deficient, and the change in my MR is precisely the kind of change which the docs have always explicitly said may happen (even if all the consequences have only been explicitly spelled out more recently).
I want to reply separately about what alternative idiom I think your tests should probably use.
@TheBlueMatt Unfortunately, it isn't possible for us to make library changes like this in an edition. Editions can affect how code is compiled, but not the underlying behavior of the standard library, partly because you can have code from Rust 2015, 2018, and 2021 all compiled into one binary and sharing the same standard library.
For what it's worth, we absolutely value stability as critically important. In this case, we made a careful tradeoff, knowing that some code out there (especially test code) might be affected. We did consider making these alternatives insta-stable, but that still would have broken your test case because the enum variant would no longer match. We also considered "never extend ErrorKind again" as a potential alternative, but we felt that because ErrorKind is #[non_exhaustive] and because we don't expect non-test code to typically match on Other in this fashion, we thought it would be reasonably safe to make this change.
One other note: #85746 (which is the change that actually affected your test) is targeted at 1.55, and has a relnotes tag; we're expecting to have a detailed release note explaining the situation for people. A release note does not ameliorate the breakage, but it should help communicate the nature and rationale and tradeoffs of the change.
In this case, we made a careful tradeoff, knowing that some code out there (especially test code) might be affected. We did consider making these alternatives insta-stable, but that still would have broken your test case because the enum variant would no longer match. We also considered "never extend ErrorKind again" as a potential alternative, but we felt that because ErrorKind is #[non_exhaustive] and because we don't expect non-test code to typically match on Other in this fashion, we thought it would be reasonably safe to make this change.
I'm curious if extending the #[non_exhaustive] API in rustc to warn around matching Other variants was considered? eg adding a tag to #[non_exhaustive] to certain variants which indicates "generate a warning if this is matched on or compared against" at compile-time so as to make this exact language pattern better supported. I understand this is maybe part of the goal of #85746, but it does feel like an oversight in the expressiveness of the #[non_exhaustive], as mentioned in the description of #85746.
I'm curious if extending the
#[non_exhaustive]API in rustc to warn around matchingOthervariants was considered? eg adding a tag to#[non_exhaustive]to certain variants ...
No, we didn't think of that.
I understand this is maybe part of the goal of #85746, but it does feel like an oversight in the expressiveness of the
#[non_exhaustive], as mentioned in the description of #85746.
Now that #85746 has landed, I don't think this situation can arise again with ErrorKind. In the future, new ErrorKinds may indeed be added, for new kinds of underlying OS errors. But when that happens, the new kind will appear instead of Unknown - not instead of Other. Unknown is permanently unstable so cannot be matched in tests (without turning on an unstable feature). So it would not be possible to write, on stable, a test which will break in the future.
I don't know if there are other enums in the stdlib with similar properties to ErrorKind. I can't think of any offhand.
No, we didn't think of that.
I do wonder if it may make sense as a language feature in the future also for non-stdlib users. It doesn't feel strange to think that some crates may want a similar pattern. I admit the 85746 approach may make more sense to them, but there is no way to non-stabilize a particular enum variant outside of std, as far as I understand.
Unknown is permanently unstable so cannot be matched in tests (without turning on an unstable feature). So it would not be possible to write, on stable, a test which will break in the future.
Ah, maybe I partially misunderstood the way 85746 operates.
So in 1.55 this has broken our test suite for remove_dir_all by leaking into stable rather than being a nightly only feature.
It seems very poor form to return an enum variant to code which cannot match against it because the compiler considers it a nightly only feature. See XAMPPRocky/remove_dir_all#33 : the failing jobs are failing in stable because the change isn't present there, but putting the change into stable code results in:
error[E0658]: use of unstable library feature 'io_error_more'
--> src/portable.rs:83:15
|
83 | &[io::ErrorKind::NotADirectory, io::ErrorKind::Other],
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
To be really clear, my beef isn't with the breakage around Other, its that the new status quo means neither the old code nor the new code works. I'd like to see that remedied ASAP, since we can't run tests at all on stable today. Ironically, the tests are one @ijackson wrote ;)
@rbtcollins, to reiterate #86442 (comment), nothing is supposed to be comparing equality to ErrorKind::Other. Code that does this is wrong.
If the error kind is not one of the stable specific variants (non-Other), the correct way for stable code to match it is _ and new unstable variants have no bearing on that as they would continue to match _.
@dtolnay Ok, sure, we can fix this, probably by matching every other error type to exclude them but the definition has clearly shifted from 1.54 and before, and this leaves a pretty poor feeling here: https://blog.rust-lang.org/2014/10/30/Stability.html made a huge impression on me, and many others, and it seems like there was debate here, but the conclusion was to modify existing behaviour, rather than allowing edition based opt-in.
The 1.54 docs were:
Other
Any I/O error not part of this list.
Errors that are Other now may move to a different or a new ErrorKind variant in the future.
This is very different to "must use _ to match" which implies must exhaustively match every other Kind to be moderately sure the desired error has occurred - which I don't look forward to doing.
Whats the right way to have a discussion about the ergonomics here? 85746 seems like a really awkward interface : theres no way that I know of to ergnomically say 'match only unstable entries'. So when one is interested in a concrete error, that is currently behind an unstable feature flag, the choices are to either accept 'any random error', or list all-known-errors as failure cases.
If your code previously just checked that the error was ErrorKind::Other, without a more detailed assert (e.g., via the os error code), deleting that test seems like the right path forward, presuming you don't want to add platform-specific code to make the test actually confirm a specific error.
I think the test you cited is a little different -- it looks like it was intending to check that the error in question is not e.g. a permissions error or similar. There it seems like it makes more sense to either move the test toward using raw_os_error and checking a specific error code (of course, platform specific) or asserting that the test is not of some collection of particular variants (e.g., if the goal is to make sure the failure is not due to permissions errors...?).
The only real change after #85746 is that it's harder for someone to assert "this is an error std didn't know enough about to differentiate", but that doesn't seem like a particularly useful assertion for tests to make -- it doesn't test a specific error, only that it's not one of a very small limited subset.
Whats the right way to have a discussion about the ergonomics here?
The right way is to counterpropose some alternative way by which new error kinds can be accommodated as operating systems and standard library implementations evolve over time.
@Mark-Simulacrum Yes, we can either take on platform specific handling, or exclude all the known variants that this isn't; and previously we could do that in a pithy fashion by matching on Other exactly.
@dtolnay obviously ;) I meant - do I carry on here, or file a new bug, or create an RFC :- I'm not sure of the most effective way to engage.
Putting some principles forward, it should be possible to both do positive assertions - error is specifically X that we know, including some X that the stdlib hasn't yet categorised. And it should be possible for existing uncategorised things to be categorised better over time.
We can either consider better over time to be at most once per edition, or up to multiple times per edition.
At most once per edition is easy to reason about; there are no strong commitments that nothing will be incompatible between editions. It might be nice to do it more often though.
lets consider the dimensions we're dealing with.
The OS adds variants without coordinating with us, so any enumeration we have will be incomplete. We need to emit those when they happen though, which gives rise to Other and now Unknown.
Callers of APIs will rarely want 234 different code paths, so matching some set of cases and then a wildcard is common. non-exhaustive can be used to require this.
Some callers will be looking for those as-yet uncategorised cases, so they would like some way to match on those. If the uncategorised cases are in a hidden/unstable variant, thats impossible. If those cases are in an accessible variant, recategorising them will be an API break, requiring either callers to accept this is as API skew over time, or for us to only do this at most once per edition. Or recategorising has to be done by some specialisation within the uncategorised bucket.
So one possibility occurs to me:
Have an IoErrorKind::Uncategorised(UnstableCategory)
UnstableCategory is an enum which grows over an edition, and the contents moved to IoErrorKind each edition.
#[non_exhaustive]
enum UnstableCategory {
Uncategorised,
...
}
Then consuming code that wants to match a currently uncategorised error:
match kind {
IoErrorKind::Uncategorised(_) => { println!("expected case"); }
_ => { panic!("unexpected error occured"); }
}
If a point release of Rust categorises that error - lets say NotADirectory, then the following happens:
#[non_exhaustive]
enum UnstableCategory {
Uncategorised,
NotADirectory,
...
}
And the user side code doesn't break. The user can opt in to tighter matching if they wish, either with some build.rs that sets a feature based on the compiler version, or a MSRV.
match kind {
IoErrorKind::Uncategorised(UnstableCategory::NotADirectory) => { println!("expected case"); }
_ => { panic!("unexpected error occured"); }
}
How is this different to the status quo? All stable Kinds are outside the space matched on, so trying to do this at the IoErrorKind layer would fail: it would bring in all known kinds, and matching just the placeholder for uncategorised kinds fails if we try to categorise directly by moving things outside of that cat4egory.
error is specifically X that we know, including some X that the stdlib hasn't yet categorised
Allowing code to match "something the standard library hasn't yet categorized" would set that code up to be broken when the standard library does start categorizing it in the future.
And it should be possible for existing uncategorised things to be categorised better over time.
That's exactly the problem: we need to make sure that existing code doesn't break when we add new categorization.
We did that by ensuring the only way to match "unknown error" is with a wildcard (_), so that when the standard library adds new categorization your code doesn't break.
We were faced with a three-way choice: never categorize any additional errors (not an option), or require people to change their code every single time we added a new categorization, or require people to change their code once if they're currently using ErrorKind::Other. And the documentation already warned against matching ErrorKind::Other in that way.
At most once per edition is easy to reason about; there are no strong commitments that nothing will be incompatible between editions. It might be nice to do it more often though.
In addition to the issue of doing it more often, we also can't tie this to an edition. io::ErrorKind can't be a different type in different editions, for a variety of reasons but in particular because one Rust program can include code from multiple editions.
Also, the change you're proposing of adding something like ErrorKind::Uncategorized is effectively what we did, and it would cause exactly the same issue with any code currently matching ErrorKind::Other.
Here's another way to look at the change:
- If you're using stable Rust, you have to use a wildcard
_to match uncategorized errors. When a new ErrorKind is added, your code will continue to work because it matches_. When that new ErrorKind is stabilized, your code will still continue to work, because it matches_. - If you're using nightly Rust, the same thing applies, and you should still use
_, but you have the option of starting to match new ErrorKind variants as soon as we add them, without waiting for them to be stabilized.
Allowing code to match "something the standard library hasn't yet categorized" would set that code up to be broken when the standard library does start categorizing it in the future.
I specifically proposed a mechanism that wouldn't break the code (within the same edition) if the stdlib started categorising it - but only sustainable for a relatively short period, because eventually the number of newly categorised errors will be large enough that people will want to match on the specific sub-variant.
re: needing the same errorkind across editions - ah, thats a constraint I hadn't internalised. If I can think of a way forward I'll suggest it, but thats a pretty hard obstacle.
Yes, I understand what has been done so far. It has removed a useful capability that existed before. I find it frustrating that most people seem to blithely assert that everything is Just Fine and that the new situation is better - only @Mark-Simulacrum has actually picked up on the impact so far.
@rbtcollins I appreciate that the mechanism you're suggesting wouldn't necessarily break code when categorization was added (though people would have to change code to match if the categorization was moved to the top level alongside other variants). But the mechanism you're suggesting would break code when the mechanism itself was introduced, in exactly the same way that was already done. With the approach that was implemented, code that matched ErrorKind::Other has to match _ instead; with the approach you're suggesting, code that matched ErrorKind::Other would have to match ErrorKind::Uncategorised(_) instead. Either way, existing code that uses ErrorKind::Other would break. The approach we used was to allow the introduction of new variants in the future without any further breakage.
And to be clear, I'm not suggesting this is an ideal solution or that it has zero impact, just that it seemed like the best of the solutions we had available.
std::io::ErrorKind::Foofor manyFoo. I will paste a list here when the MR is merged :-).
PR has been merged for a while ;)
Hey, I was wondering if these extra error kinds are added in std lib for stable or not?
I am using rustc 1.56.1 and trying to create a file in a directory (using fs::OpenOptions) gives me ReadOnlyFilesystem error,(which is expected as the fs is supposed to be readonly,) but if I try error.kind() == std::io::ErrorKind::ReadOnlyFilesystem
I get compiler error use of unstable library feature.
Is there a way I can check if the error I get is of this specific kind without using nightly?
The way I understood rust release process I thought unstable features were nightly -> beta -> stabilized , but in this case it seems half of this is in stable and half is still unstable? Apologies if I misunderstood anything.
To elaborate, my current setup is
if kind == ro_fs{
// do something
}else{
// do something else
}I am compiling with rustc v 1.56.1, for target x86_64-unknown-linux-gnu, with rustflags -C target-feature=+crt-static on Linux OS.
Thanks :)
EDIT/UPDATE : on reading the discussion above once again, I figured out I can use the same way that TheBlueMatt has used and use raw_os_error, so that might work in my case. Thank you.
@joshtriplett well, I'm finally getting time to do any rust, and the first thing is fixing up the still broken tests in remove_dir_all, and that means figuring this out. 6 months later and its still not usable on stable, so I'll need to use raw os errors indefinitely.
I still maintain that this is a stability break, no matter how desirable the feature. https://blog.rust-lang.org/2014/10/30/Stability.html
Code that worked, that was written with the API of the time, now doesn't work. We've pushed work onto our users.
We should update the stability guarantee I think - it was inspirational, but we're not living up to it.
/end whinge
@rbtcollins I think you should use raw OS errors, yes.
I think this is an opportunity to plug my blog post about this issue where I go into some detail about what the problems are, why there aren't easy answers, and what to do if you're trying to fix things.
In #90706 I proposed to add a link to my blog post. That was felt to be inappropriate. Instead in #94273 a very abbreviated version of my recommendation was added. I still think this is a complicated and difficult issue and it is a shame that the official documentation still doesn't really reference a thorough explanation for people in @rbtcollins's situation, who are quite understandably frustrated.
I wonder if adding to that prose a comment about raw errors would be useful?
I think there are three cases:
One knows the error, and the version of rust at the time the code is written knows it: match on exactly that error.
One knows the error, and the version of rust at the time the code is written does not know it: use raw os errors and platform specific code to match on exactly that error.
One doesn't know the exact error: match on the inverted set of all the errors you know it cannot be (and if some of those are not known to rust, perhaps do so with raw os errors).
Personally, I want this feature to get stabilized.
There is no way for a lib to construct an error like IsADirectory with error context in stable rust as described in apache/opendal#221.
I am running into some similar problems. It seems that std::fs::read_to_string can potentially return an Err<std::io::Error> with ErrorKind::IsADirectory as its .kind().
I can use a _ wildcard match to handle it and display a generic error (eg. eprintln!("{:?} IO Error", kind)), but even though doing that will print "IsADirectory IO Error" to stderr on stable, I can't actually match against it on stable.
That this feature exists is becoming annoying in my everyday work, because my editor will suggest that I use (for instance) InvalidFilename and then turn around and tell me I'm not allowed to use InvalidFilename because the feature isn't stabilized. I rarely express the opinion that an unstable feature is somehow harmful but, in this case, I'm starting to feel it's time to "act" or get off the pot, so to speak.
@archer884 considering there are other unstable APIs in std, that seems more like an issue for editor plugins rather than motivation to stabilize or remove this nightly feature. Ideally completions would hint if something is unstable or hide them entirely (either on stable toolchain or configured to hide). There are open issues to improve this situation in rust-analyzer and intellij-rust, such as rust-lang/rust-analyzer#3020 and intellij-rust/intellij-rust#4439.
@archer884 considering there are other unstable APIs in std, that seems more like an issue for editor plugins rather than motivation to stabilize or remove this nightly feature. Ideally completions would hint if something is unstable or hide them entirely (either on stable toolchain or configured to hide). There are open issues to improve this situation in
rust-analyzerandintellij-rust, such as rust-lang/rust-analyzer#3020 and intellij-rust/intellij-rust#4439.
As I mentioned here, this affects my Rust code itself, independent of my IDE. Functions such as std::fs::read_to_string are able to return errors (on stable) that are a part of the io_error_more unstable API, but they can't be directly matched against (except with a wildcard match) because they are unstable. But I can call methods like .to_string(), which work on the unstable variants. (eg. err_object.kind().to_string() can output "is a directory", even though ErrorKind::IsADirectory is unstable)
Proof of concept using evcxr_repl:
>> // The root directory `/` is always not a file. This example may not work on Windows.
.. if let Err(e) = std::fs::read_to_string("/") {
.. println!("{}", e.kind().to_string()); // `e.kind()` will be a `std::io::ErrorKind::IsADirectory`
.. }
is a directory
()
>> std::io::ErrorKind::IsADirectory
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
use of unstable library feature 'io_error_more'I'm sure that you know way more about Rust then I do, but to me it seems weird that even if #![feature(io_error_more)] is not enabled it can still exist in a limbo state where it can't be referenced directly but can be used.
I'm sure that you know way more about Rust then I do, but to me it seems weird that even if
#![feature(io_error_more)]is not enabled it can still exist in a limbo state where it can't be referenced directly but can be used.
@superatomic apologies if I wasn't clear. I was addressing a general (not unique to this feature) issue about editor suggestions of unstable parts in std, but I wasn't implying what should be done with #![feature(io_error_more)]. As you and others note, there are issues specific to this feature that motivate actions such as stabilization.
I also want to vouch for stabilization as soon as possible. The existence of these unstable enum variants would be fine, if the creation of them was also "unstable only". Right now the unstable variants can be created in the internals of std in stable, without being able to create these same variants externally in stable. It makes mocking APIs, testing error handling and various other things very challenging.
We are going to need to implement some rather elaborate io::Error wrapping systems in the Deno project now to be able to manually create io::Error-likes for the unstable ErrorKind variants.
Ooh, this is a fun workaround for creating unstable ErrorKinds, in stable: std::io::Error::from_raw_os_error(libc::EISDIR). Very hacky :D
Considering the ErrorKind can already be created (and matched via the stringified form) in stable, this may aswell be stable already. Updating the behavior here would likely break a considerable amount of existing applications which may unintentionally rely on these unstable error kinds already.
It seems like most of the recent comments are only about IsADirectory. Since that makes clear it's a useful and wanted error kind, it might make sense to send a PR for stabilizing that one specifically. Then that doesn't have to be blocked on the other variants being stabilized.
The existence of these unstable enum variants would be fine, if the creation of them was also "unstable only".
Any uncategorized error is unstable in the same way, regardless of whether its ErrorKind is ErrorKind::Uncategorized or a specific unstable one like ErrorKind::IsADirectory. That we internally already (experimentally) split off IsADirectory from Uncategorized shouldn't make any difference for Rust programs, as the result is the same: An io::Error with an unmatchable ErrorKind. (Which is exactly the intention, since we haven't categorized them yet, no one should match on them, as that would break once we do put them in a stable category/kind.)
It seems like most of the recent comments are only about IsADirectory. Since that makes clear it's a useful and wanted error kind, it might make sense to send a PR for stabilizing that one specifically. Then that doesn't have to be blocked on the other variants being stabilized.
That'd be great. We care mostly about IsADirectory, NotADirectory, and FilesystemLoop for our use cases right now.
It seems like most of the recent comments are only about
IsADirectory. Since that makes clear it's a useful and wanted error kind, it might make sense to send a PR for stabilizing that one specifically. Then that doesn't have to be blocked on the other variants being stabilized.
I support stabilizing IsADirectory, but I highly suggest that we stabilize NotADirectory at the same time as they are opposites of each other. (like @lucacasonato said)
It might also be worth stabilizing DirectoryNotEmpty as well, but it doesn't appear as important and is only partially related (it's the only other directory-related error, but it doesn't deal with the existence of directories).
The existence of these unstable enum variants would be fine, if the creation of them was also "unstable only".
Any uncategorized error is unstable in the same way, regardless of whether its ErrorKind is ErrorKind::Uncategorized or a specific unstable one like ErrorKind::IsADirectory. That we internally already (experimentally) split off IsADirectory from Uncategorized shouldn't make any difference for Rust programs, as the result is the same: An io::Error with an unmatchable ErrorKind. (Which is exactly the intention, since we haven't categorized them yet, no one should match on them, as that would break once we do put them in a stable category/kind.)
Is there some way to prevent or warn about the use of ErrorKind::Other and ErrorKind::Uncategorized? Perhaps a Clippy lint like @ijackson was talking about might make sense.
Returning unstable variants is still not ideal even if we should treat them like ErrorKind::Other in stable code and just use wildcards. I think the biggest potential problem is that returning unstable varients implies that they are stable (i.e. matching ErrorKind::Other is bad form, but still possible). It might not be ideal to change which variant is returned depending on whether #![feature(io_error_more)] is used, but it might be worth only splitting errors off from ErrorKind::Uncategorized when io_error_more is enabled, instead of always.
Is there some way to prevent or warn about the use of ErrorKind::Other and ErrorKind::Uncategorized?
Using Uncategorized already gives an error:
error[E0658]: use of unstable library feature 'io_error_uncategorized'
--> src/main.rs:2:13
|
2 | std::io::ErrorKind::Uncategorized;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Or do you mean something else?
I think the biggest potential problem is that returning unstable varients implies that they are stable
I'm not sure I understand. ErrorKind is #[non_exhaustive], so you'll always have to take into account that it might have more variants than you can't match on. Whether some of those have an unstable/experimental name or not shouldn't make any difference.
change which variant is returned depending on whether
#![feature(io_error_more)]is used
What would happen if an (indirect) dependency enables that feature, but your own crate does not? (Or the other way around?) Especially if that dependency ends up creating an Error or ErrorKind to your crate. An entire Rust program (which can be a combination of many crates) uses a single copy of std, so we can't make std behave differently per crate.
I prefer to stabilize the following error kind:
NotADirectoryIsADirectoryDirectoryNotEmpty
The reasons are listed as follows:
They are normal
Any call to remove_dir or create_dir could trigger them unless we check metadata every time we try to call std::fs functions.
They are actionable
Developers have a strong need to check and match them.
Think about we are developing command-line tools. We may want to output friendly messages if users' input path is a directory.
References:
The only thing I want to know before stabilizing any of them, is if they make sense on all/most platforms. E.g. ideally, trying to remove a non-empty directory would result in the same DirectoryNotEmpty error kind on all platforms.
This doesn't necessarily have to be a strict requirement. There might be cases where one OS simply does not have errors that are specific enough. But in those cases we should be extra careful, and wonder if we're making our error kinds too specific.
The only thing I want to know before stabilizing any of them, is if they make sense on all/most platforms. E.g. ideally, trying to remove a non-empty directory would result in the same DirectoryNotEmpty error kind on all platforms.
Nice catch. The three errors are obvious on posix platforms like linux, darwin, and bsd.
Windows also have some compatibility with the UNIX family of operating systems, which is the subset of errno.h.
- NotADirectory => ENOTDIR
- IsADirectory => EISDIR
- DirectoryNotEmpty => ENOTEMPTY
References: https://docs.microsoft.com/en-us/cpp/c-runtime-library/errno-constants?view=msvc-170
Is there some way to prevent or warn about the use of ErrorKind::Other and ErrorKind::Uncategorized?
Using
Uncategorizedalready gives an error:error[E0658]: use of unstable library feature 'io_error_uncategorized' --> src/main.rs:2:13 | 2 | std::io::ErrorKind::Uncategorized; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^Or do you mean something else?
Oops, I didn't check to see if it already issued a warning. ErrorKind::Uncategorized is not a problem.
Should ErrorKind::Other behave in the same way, though?
Currently, Rust doesn't warn about using it, even when it used in a pattern match.
I think the biggest potential problem is that returning unstable variants implies that they are stable
I'm not sure I understand.
ErrorKindis#[non_exhaustive], so you'll always have to take into account that it might have more variants than you can't match on. Whether some of those have an unstable/experimental name or not shouldn't make any difference.
Sorry, I should have been clearer about what I was talking about.
If a programmer understands that the unstable ErrorKind enum variants should be treated as ErrorKind::Other and only matched via wildcards, there isn't a problem.
However, because these variants are given specific names and descriptions, and return based on certain contexts, it gives off the appearance of being usable.
Basically, it's not immediately obvious that the unstable variants should be ignored and simply wildcard matched.
change which variant is returned depending on whether
#![feature(io_error_more)]is usedWhat would happen if an (indirect) dependency enables that feature, but your own crate does not? (Or the other way around?) Especially if that dependency ends up creating an Error or ErrorKind to your crate. An entire Rust program (which can be a combination of many crates) uses a single copy of
std, so we can't makestdbehave differently per crate.
I didn't know that std worked like that. I agree, it shouldn't return different variants. Sorry!
This issue is really starting to bug me as it breaks some existing code and, as others have said, prevents from fixing it.
I have a specific scenario where I need to move a file to a different filesystem, the only reliable cross-platform way I've found to do it is to fs::rename and check the resulting error.
Now this doesn't work because the ErrorKind:CrossesDevice variant has been introduced. I don't like silent breakages like this one but I understand how they are necessary.
The problem I have is that the code can't be fixed, I now need to check for this specific error variant but I can't because the feature is unstable and won't compile by definition on stable. So my program doesn't work anymore and I don't have any reliable way to make it work without using some huge hacks that may completely break in the future.
I don't know what solution could be brought - I thought about adding a function that would convert the new variants to the old one, but it would of course be unstable for a bit of time so that wouldn't solve the problem at all.
Do you have any idea of improvements to solve this problem?
First of all, as advised by @m-ou-se, we are on the way to stabilizing part of io_error_more instead, like NotADirectory and DirectoryNotEmpty. This can prevent us kept blocking without any progress. And we will not stabilize ErrorKind:CrossesDevice for now. (Please correct me if I understand wrongly).
Then, for this problem:
I need to move a file to a different filesystem, the only reliable cross-platform way I've found to do it is to fs::rename and check the resulting error.
Which variant do you check before?
As described in docs,
Errors from the standard library that do not fall under any of the I/O error kinds cannot be matched on, and will only match a wildcard (_) pattern. New ErrorKinds might be added in the future for some of those.
To me, the clearest way is falling back to copy&delete (just like mv does) while meeting errors except NotFound, PermissionDenied, and others already known (and stable) error variants.
Currently, Rust doesn't warn about using it, even when it used in a pattern match.
Maybe it's worth adding a Clippy rule for matching ErrorKind::Other.
Proposed new lint at rust-lang/rust-clippy#9004
Nice answer, thank you.
May I ask why ErrorKind isn't marked as #[non_exhaustive] given new variants can be added anytime and ErrorKind::Other shouldn't be matched against?
May I ask why
ErrorKindisn't marked as#[non_exhaustive]given new variants can be added anytime andErrorKind::Othershouldn't be matched against?
ErrorKind does marked as #[non_exhaustive]:
Reference: https://doc.rust-lang.org/std/io/enum.ErrorKind.html
Oh yes sorry, I confused #[non_exhaustive] and having a non-matchable variant.
Nevermind.
This doesn't necessarily have to be a strict requirement. There might be cases where one OS simply does not have errors that are specific enough. But in those cases we should be extra careful, and wonder if we're making our error kinds too specific.
Highly relevant to this concern is the discussion starting here: #79965 (comment)
In particular, I would like to (re)propose the principles I set out here #79965 (comment)
The only thing I want to know before stabilizing any of them, is if they make sense on all/most platforms. E.g. ideally, trying to remove a non-empty directory would result in the same DirectoryNotEmpty error kind on all platforms.
I think what you mean is whether there are platforms where you can try to remove a non-empty directory, and get an error because it was non-empty, and not be able to tell that that was the reason ?
I think if there are any such operating systems then they ought to either not implement these parts of std, or ought to be at a low support tier where it is expected that Rust programmers might have to do platform-specific work.
The alternative is to say that all Rust programmers even on popular and sensible operating systems must apply platform-specific workarounds (involving raw OS error numbers), or write unnatural code, or be exposed to toctou races.
I would like to propose that we stabilize these. Based on the feedback above, these are very useful to have in stable, and there seem to be no real risks. Concretely, this would mean marking the following as stable:
io::ErrorKind::HostUnreachableio::ErrorKind::NetworkUnreachableio::ErrorKind::NetworkDownio::ErrorKind::NotADirectoryio::ErrorKind::IsADirectoryio::ErrorKind::DirectoryNotEmptyio::ErrorKind::ReadOnlyFilesystemio::ErrorKind::FilesystemLoopio::ErrorKind::StaleNetworkFileHandleio::ErrorKind::StorageFullio::ErrorKind::NotSeekableio::ErrorKind::FilesystemQuotaExceededio::ErrorKind::FileTooLargeio::ErrorKind::ResourceBusyio::ErrorKind::ExecutableFileBusyio::ErrorKind::Deadlockio::ErrorKind::CrossesDevicesio::ErrorKind::TooManyLinksio::ErrorKind::InvalidFilenameio::ErrorKind::ArgumentListTooLong
The issues around io::ErrorKind::Other and the unexpected failures they caused have been resolved with the introduction of io::ErrorKind::Uncategorized. This also eliminates the potential breakage from exhaustive matching. As such, stabilization itself should not be a breaking change anymore.
As to the concern raised by @m-ou-se, I have gone over all of these errors, and almost all of them seem to have compatible OS error conditions on both windows and any posix compliant system. As such, platform specific issues should only occur on unusual platforms outside of either the current tier 1 or tier 2 lists. I think we should be OK treating any cases where these errors are not generated currently as bugs. A further search of the current open issue list suggests no such issues are currently known.
There are two exceptions:
First, io::ErrorKind::FilesystemQuotaExceeded and io::ErrorKind::FileTooLarge do not seem to correspond to posix standardised errors. I think for these in particular it is acceptable if they fall back to io::ErrorKind::StorageFull, which seems to be what the posix standard suggest would happen under those conditions.
Second, io::ErrorKind::ExecutableFileBusy and io::ErrorKind::Deadlock seem to be best effort, and have limited system support, but this seems to me well enough documented.
io::ErrorKind::FileTooLarge corresponds to EFBIG, which is in POSIX, see here. This error is distinct from io::ErrorKind::StorageFull or ENOSPC.
As this problem is still around, I wonder if it would be a good idea to think about how to handle such changes.
Concretely, this is NOT a breaking change but a functional change, which is as important but not covered the same by Rust's guarantees.
The problems I see here are that:
- It isn't really clear what Rust offers as guarantees for breaking functional changes such as this one (it was documented in the 1.55's release notes), as they can still break stable code if you don't pay attention to the changelog. And we aren't talking about using an
unsafeor nightly feature, but one that has been available for a long time. - This also means that potentially any item of
stdandcorecould be affected by this problem. This makes writing reliable and future-proof code a lot harder and require to read every single changelog when upgrading to a newer Rust compiler. - Even after this breaking change, there doesn't seem to be a reliable and future-proof way to correctly handle these IO checks, which is worrysome. Especially given more IO variants could happen to be released in the future.
I don't know what we could about these, but I think clarifying at least Rust's guarantees for functional changes could help (maybe it's already specified somewhere but I didn't find it). And officially endorsing that such breaking changes will ALWAYS be highlighted in Rust's release notes (like it has been for the 1.55's blog post).
Sorry, I don't follow how this wasn't a breaking change. It was acknowledged as such much earlier on in the discussion. Code that did compile stopped compiling.
Sorry, I don't follow how this wasn't a breaking change. It was acknowledged as such much earlier on in the discussion. Code that did compile stopped compiling.
Sorry it wasn't very clear in my message, I mean that it didn't change nor remove any entity (variable, function, trait, module) or their signature.
This is a functional change in the sense that the ErrorKind variants returned by the IO functions aren't the same as before, no variant was removed or renamed, it's just that these functions don't return the same values anymore.
I think, at this point, we would provide the best experience by stabilizing these, so that we don't return errors people can't match.
@rfcbot merge
Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:
No concerns currently listed.
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!
See this document for info about what commands tagged team members can give me.
I think, at this point, we would provide the best experience by stabilizing these
I very much agree that we should stabilise these.
Having said that, I can understand why it has taken a while.
The conversations in this area have been very long and detailed. The change from Other to Unrecognized, which has already been done, and is not part of what we're proposing to stabilise, has (perhaps inevitablh) become entangled in the conversations (especially since it was prompted by the proposal to add new errorkinds).
Also, this is an area that many people interact with on a daily basis but which also has quite surprising depths in a number of diifferent directions. So people tend to come with questions. I think we have answered all the questions satisfactorily - but it is not always to find the answers in these long threads.
If necessary (eg, if we don't have consensus for the FCP here) maybe I could write up a stabilisation report or something maybe.
I'm expecting Unrecognized to remain perma-unstable, by way of avoiding the problem that occurred with Other.
And yes, I think there have been a lot of questions, but I do think they've been answered. Anyone is free to call out questions that haven't been, though.
Shouldn't this be a libs-api FCP?
Shouldn't this be a libs-api FCP?
Probably, yes, as I understand it.
Anyone is free to call out questions that haven't been, though.
Absolutely.
I'm expecting
Unrecognizedto remain perma-unstable, by way of avoiding the problem that occurred withOther.
Yes.
Going to cancel the fcp in progress because it was started under the wrong team. I haven't refreshed myself on the current state of the thread yet so I don't want to start the FCP since it will permanently check my box, @joshtriplett please restart the FCP under libs-api whenever you have a chance.
@rfcbot cancel
@rfcbot merge
Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:
No concerns currently listed.
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!
See this document for info about what commands tagged team members can give me.
Sorry, I don't follow how this wasn't a breaking change. It was acknowledged as such much earlier on in the discussion. Code that did compile stopped compiling.
Sorry it wasn't very clear in my message, I mean that it didn't change nor remove any entity (variable, function, trait, module) or their signature.
The breaking change was the change in possible match values ErrorKind from exhaustive, to non-exhaustive. Thats part of the signature is it not?
This is a functional change in the sense that the
ErrorKindvariants returned by the IO functions aren't the same as before, no variant was removed or renamed, it's just that these functions don't return the same values anymore.
It is true that Other wasn't removed, but even with no change to return values, this would be breaking, because non-exhaustive is an attribute of the enum type.
I'm pulling on this thread because I think its important that this be classified as a breaking change, not as a 'functionally breaking change'.
A change in what variant is returned under some circumstances is a change that is only detected at runtime. So yes, that would be a functional breaking change.
This was detected at compile time. => a breaking change.
@rbtcollins it sounds like you've said a couple times you think std::io::ErrorKind got changed from being exhaustively matchable to not being exhaustively matchable. That is not the case. ErrorKind has never been exhaustively matchable in stable code in any existing release of Rust.
@dtolnay ahha. I had misremembered how the change happened. Thank you for helping me get clear on it. Sorry @ClementNerma for the unneeded contention.
I would like to clarify once again that the Other->Unrecognized change, which did in practice cause things to go wrong for a significant number of people, regardless of what kind of breakness we decree it to have had, has already happened and is not part of this FCP.
What we are proposing to stabilise now is just the ability to name the additional ErrorKinds (IsADirectory etc.) which are already being generated, currently-stably-unmatchably, So if this FCP passes it will not make any breakage or trouble worse in any way. Indeed, ability to match these variants is likely to be practically useful to (belatedly) properly fix some of the downstream test suites that suffered regressions. I appreciate that the introduction of Unrecognized was confusing and controversial and awkward, but it is not to do with this FCP (nor indeed to do with this MR - it was done in some other MR).
I think the primary questions that ought to be asked apropos this FCP are:
- Are the error classifications implied by the new
ErrorKindsthe right ones? Specifically: - Are the distinctions we make here the ones that users need?
- Is this likely to be implementable on future platforms we might want to support? This is subtle: what we need is not that these errors correspond to errors on all platforms. Rather, we need for our classification not to make distinctions between situations that (possibly hypothetical) reasonable platforms undistinguishably conflate. So platform-specific
ErrorKinds are fine if Rust programmers need to be able to match them in portable code. - Do they have the right names?
I think the answers to these questions are "yes". We spent the bulk of the discussions in this MR about the details of individual errors, with a smattering of more general discussions about principles. The MR had a lot of attention from Unix and Windows experts and there has been a year for folks from other systems to chime in.
But, of course, we might have missed something, which is what the FCP is for.
What we are proposing to stabilise now is precisely the ErrorKinds listed in this table.
I have included the Unix and Windows errno values that map to these kinds. We are stabilising these mappings, but if we have missed some errors that ought to be categorised into these kinds and are currently Uncategorized, I think we could add those mappings later and it would be at worst a minor functional breakage.
| ErrorKind | Unix | Windows |
|---|---|---|
| ArgumentListTooLong | E2BIG | |
| CrossesDevices | EXDEV | ERROR_NOT_SAME_DEVICE |
| Deadlock | EDEADLK | ERROR_POSSIBLE_DEADLOCK |
| DirectoryNotEmpty | ENOTEMPTY | ERROR_DIR_NOT_EMPTY |
| ExecutableFileBusy | ETXTBSY | |
| FileTooLarge | EFBIG | ERROR_FILE_TOO_LARGE |
| FilesystemLoop | ELOOP | |
| FilesystemQuotaExceeded | EDQUOT | ERROR_DISK_QUOTA_EXCEEDED |
| HostUnreachable | EHOSTUNREACH | ERROR_HOST_UNREACHABLE WSAEHOSTUNREACH |
| InvalidFilename | ENAMETOOLONG | ERROR_FILENAME_EXCED_RANGE ERROR_INVALID_NAME |
| IsADirectory | EISDIR | ERROR_DIRECTORY_NOT_SUPPORTED |
| NetworkDown | ENETDOWN | WSAENETDOWN |
| NetworkUnreachable | ENETUNREACH | ERROR_NETWORK_UNREACHABLE WSAENETUNREACH |
| NotADirectory | ENOTDIR | ERROR_DIRECTORY |
| NotSeekable | ESPIPE | ERROR_SEEK_ON_DEVICE |
| ReadOnlyFilesystem | EROFS | ERROR_WRITE_PROTECT |
| ResourceBusy | EBUSY | ERROR_BUSY |
| StaleNetworkFileHandle | ESTALE | |
| StorageFull | ENOSPC | ERROR_DISK_FULL ERROR_HANDLE_DISK_FULL |
| TooManyLinks | EMLINK | ERROR_TOO_MANY_LINKS |
@ijackson for ELOOP, Windows appears to give winapi::shared::winerror::ERROR_CANT_RESOLVE_FILENAME in similar situations (e.g. symlink loops). Could we add that in, or perhaps generalise FileSystemLoop to the slightly more general case of being unable to resolve, before stabilisation?
I'm marking @yaahc's box because she has stepped down from T-libs-api after the point that this feature got proposed for FCP.
๐ This is now entering its final comment period, as per the review above. ๐
@rbtcollins What other things might ERROR_CANT_RESOLVE_FILENAME mean? If it might mean other kinds of things besides loops then I think Windows fails to reliably distinguish filesystem loops from other kinds of errors. Which might mean that portable Rust programs aren't allowed to expect the operating system to reliably distinguish loops from other problems :-/.
@rfcbot concern Windows ERROR_CANT_RESOLVE_FILENAME, FilesystemLoop
If we can't resolve this question immediately, I will remove FilesystemLoop from the to-be-stablised list and we will have to deal with that one later. I don't want to block all the rest.
(Maybe someone with some rfcbot permissions could note the concern. This MR shouldn't merge without this being resolved, if only by dropping the stabilisation of that one kind.)
Oh wait this isn't a stabilisation MR - it's just an issue, so we can just make the MR not include FilesystemLoop. But it would be good to be clear whether we are doing that.
The win32 error ERROR_CANT_RESOLVE_FILENAME currently maps only from the kernel error STATUS_REPARSE_POINT_NOT_RESOLVED.
The kernel error has the English error message as "The symbolic link could not be resolved even though the initial file name is valid". I'm pretty sure this situation can only occur when a loop is hit. EDIT: Or a long sequence of links that's not a loop but its length is greater than whatever the maximum is.
However, the win32 error message is more generic "The name of the file cannot be resolved by the system" so I guess the argument can be made that it does allow for more errors to be mapped to it in the future, should the need arise. I'm not sure that that would ever happen but I'll admit it is at least a possibility.
some system calls on Linux also use ELOOP to mean "ELOOP A loop exists in symbolic links encountered during resolution of the path argument, or O_NOFOLLOW was specified and the path argument names a symbolic link." so I think interpreting it as "symlink loop or similar symlink resolve error was encountered" might be an accurate description, although (bike-shedding!) I don't know if FilesystemLoop is an accurate name then, and not something like SymlinkResolutionFailed or such...
I think NotSameDevice is a better name than CrossesDevices. For me, it expresses its meaning more clearly. FWIW, the Linux man page also uses the word "same" to describe EXDEV: oldpath and newpath are not on the same mounted filesystem.
Also, I find NotSameDevice to be stylistically more consistent with the other existing and proposed error identifiers.
Can FilesystemQuotaExceeded be renamed QuotaExceeded? AFAIK Linux has no other quota error and EDQUOT is also used generically (not only for filesystem quotas). A few other uses are visible here: https://elixir.bootlin.com/linux/v4.5/ident/EDQUOT. The comment in errno.h simply describes it as "Quota exceeded".
I think there's a good correspondence @ijackson - as @ChrisDenton says the current meaning of ERROR_CANT_RESOLVE_FILENAME maps well to the loop meaning of ELOOP, and as @zseri has said, ELOOP itself isn't returned solely when loops are detected. Add to that list mount(2) returning ELOOP for move operations where the target is a child of the source - something that has absolutely nothing to do with symlinks, and execve returning ELOOP for exceeding recursion limits during recursive script execution (since Linux 3.8).
- because OS errors are moving targets, we cannot assume Linux / BSD / others will not introduce a 5th or 6th meaning, and its clear to me at least that Linux doesn't treat ELOOP as a filesystem error but a more general error.
I suggest renaming it to LoopError, but document that it means ELOOP on Linux and ERROR_CANT_RESOLVE_FILENAME on Windows, and either describe what we know right now, or provide breadcrumbs for readers to catch up.
The final comment period, with a disposition to merge, as per the review above, is now complete.
As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.
This will be merged soon.
I might be a little too late, but I'm not sure combining ERROR_FILENAME_EXCED_RANGE and ERROR_INVALID_NAME into InvalidFilename is a good idea. What was the reason behind this decision?
For example if one is writing an unarchiver to extract files, if a filename is too long one could simply truncate the filename and tell the user this. But if the filename is invalid/reserved (eg. COM) then the name would have to be completely different.
An app could guess which of the two it is, but it would be more straightforward if the two are distinguished from the start.
It is a bit late but technically not too late; the stabilization PR has yet to be merged.
For example if one is writing an unarchiver to extract files, if a filename is too long one could simply truncate the filename and tell the user this. But if the filename is invalid/reserved (eg. COM) then the name would have to be completely different.
I could definitely see the two cases being handled differently in some situations.
An app could guess which of the two it is, but it would be more straightforward if the two are distinguished from the start.
Note that the original error is recoverable using raw_os_error(). Though that isn't particularly ergonomic and is of course platform specific.
I was speculating there might be an OS that returns "Invalid Filename" when the filename is too long.
In which case, this decision will make sense.
But otherwise, "Filename Too Long" is one of the few errors that seem to be consistent among platforms.
With current PR, Unix doesn't have InvalidFilename mapped to anything else (at least from my quick skim through sys/unix/mod.rs).
However, UNIX-like OS fails with varying error when invalid UTF-8 filename is used to open() in C.
Mac APFS fails with EILSEQ (Illegal byte sequence).
Linux EXT4 (formatted with strict case-insensitive option -O casefold -E encoding_flags=strict, because otherwise any byte sequence is allowed) fails with EINVAL(invalid argument).
In theory, I think these should be mapped to InvalidFilename in the context of File::open(), File::create(), etc. But I understand that that would need major rewrite.
I don't know enough to say what would be the best way, but current use of InvalidFilename doesn't feel optimal.
PS. I've never contributed to rust dev. so I'm not sure of the procedures if this should get reviewed before standardization
PS. I've never contributed to rust dev. so I'm not sure of the procedures if this should get reviewed before standardization
I'll make a note on the stabilization PR that there's still some concern around the current mappings. Maybe the libs-api team will want to look at this again.
Currently io::Error stores OS errors internally as essentially OsError(i32) so there's no context available when getting the io::ErrorKind.
I think that a new error variant for the EMFILE error code on Unix platforms could be useful for embarrassingly parallel applications. I would like to contribute a PR for such a new variant, would it be a welcome addition?
Is it blocked on t-libs-api? I feel like thatโs an important feature since itโs really hard to avoid TOCTOU without it, e.g.
if let Err(err) = fs::remove_file(path) {
if matches!(err, io::ErrorKind::IsADirectory) { ... }
}becomes something racy like
if !path.is_dir() {
fs::remove_file(path)?;
}Maybe some less-controversial subset could be stabilized sooner? Whatโs blocking this issue from making progress?
one additional case that would be nice for this is "Too many open files" (EMFILE, 24 on linux)
i ran into this when writing async code, and there doesn't seem to be any way to test it besides checking the message.
@lolbinarycat On Unix-like you can do .raw_os_error() to get an error code + nix::errno::Errno for a portable list of errnos.
I would like to propose that we stabilize these. Based on the feedback above, these are very useful to have in stable, and there seem to be no real risks. Concretely, this would mean marking the following as stable:
* `io::ErrorKind::HostUnreachable` * `io::ErrorKind::NetworkUnreachable` * `io::ErrorKind::NetworkDown` * `io::ErrorKind::NotADirectory` * `io::ErrorKind::IsADirectory` * `io::ErrorKind::DirectoryNotEmpty` * `io::ErrorKind::ReadOnlyFilesystem` * `io::ErrorKind::FilesystemLoop` * `io::ErrorKind::StaleNetworkFileHandle` * `io::ErrorKind::StorageFull` * `io::ErrorKind::NotSeekable` * `io::ErrorKind::FilesystemQuotaExceeded` * `io::ErrorKind::FileTooLarge` * `io::ErrorKind::ResourceBusy` * `io::ErrorKind::ExecutableFileBusy` * `io::ErrorKind::Deadlock` * `io::ErrorKind::CrossesDevices` * `io::ErrorKind::TooManyLinks` * `io::ErrorKind::InvalidFilename` * `io::ErrorKind::ArgumentListTooLong`The issues around
io::ErrorKind::Otherand the unexpected failures they caused have been resolved with the introduction ofio::ErrorKind::Uncategorized. This also eliminates the potential breakage from exhaustive matching. As such, stabilization itself should not be a breaking change anymore.As to the concern raised by @m-ou-se, I have gone over all of these errors, and almost all of them seem to have compatible OS error conditions on both windows and any posix compliant system. As such, platform specific issues should only occur on unusual platforms outside of either the current tier 1 or tier 2 lists. I think we should be OK treating any cases where these errors are not generated currently as bugs. A further search of the current open issue list suggests no such issues are currently known.
There are two exceptions: First,
io::ErrorKind::FilesystemQuotaExceededandio::ErrorKind::FileTooLargedo not seem to correspond to posix standardised errors. I think for these in particular it is acceptable if they fall back toio::ErrorKind::StorageFull, which seems to be what the posix standard suggest would happen under those conditions. Second,io::ErrorKind::ExecutableFileBusyandio::ErrorKind::Deadlockseem to be best effort, and have limited system support, but this seems to me well enough documented.
Something I noticed that https://doc.rust-lang.org/stable/src/std/sys/pal/unix/mod.rs.html#261 uses HostUnreachable as the output of kind() however unless you're nightly you can't actually match to this since that variant is feature guarded. This is unfortunate since that's the kind that's returned by TcpStream when a host is offline. Thus when I'm trying to match this error type to attempt a retry, I can't unless I search for the code instead, not as convenient. This list needs to make its way to stable ASAP if its already being used to report errors. Whats worse is that this was done 3 years ago and no one has since complained that they can't match this kind w/o nightly?!