Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

std::io functions should document which ErrorKinds they can return #40322

Open
durka opened this issue Mar 7, 2017 · 20 comments
Open

std::io functions should document which ErrorKinds they can return #40322

durka opened this issue Mar 7, 2017 · 20 comments
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. P-medium Medium priority T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@durka
Copy link
Contributor

durka commented Mar 7, 2017

See this reddit post where the question was "How can File::open fail?" and the information is sort of there in the docs, but I was reduced to guessing io::ErrorKind variants from the text description of errors. Similar to how Unix man pages list exactly which errno values are possible, the std::io functions should list the exact variants they might return.

@retep998
Copy link
Member

retep998 commented Mar 7, 2017

More specifically it should document which system error codes correspond to which ErrorKind.

@durka
Copy link
Contributor Author

durka commented Mar 7, 2017

I guess so, but that's system-dependent. We can say that File::open fails with NotFound even if that is a different code on Windows vs Linux or whatever.

@nagisa
Copy link
Member

nagisa commented Mar 8, 2017

even if that is a different code on Windows vs Linux or whatever.

I do not think it is in our interest to maintain descriptions of every possible failure. We already document what platform API some call corresponds to, so referring people to corresponding documentation seems entirely fine to me

@durka
Copy link
Contributor Author

durka commented Mar 8, 2017 via email

@durka
Copy link
Contributor Author

durka commented Mar 8, 2017 via email

@steveklabnik steveklabnik added A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools P-medium Medium priority and removed A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools labels Mar 10, 2017
@steveklabnik
Copy link
Member

triage: p-medium. putting it on the next doc team agenda

@steveklabnik
Copy link
Member

steveklabnik commented Mar 22, 2017

Docs team discussed this today. What we plan to do is basically what the original post says; but we want to make sure that we do it in a way that suggests common errors rather than hard-specifying that it can only return one of them. The practical consideration is that the "common" errors will not be the exhaustive list, but that doesn't paint anyone into a corner later.

This was referenced Mar 24, 2017
tbu- added a commit to tbu-/rust that referenced this issue Jun 27, 2017
Try to make clear that this isn't an API guarantee for now, as we likely
want to refine these errors in the future, e.g. `ENOSPC` "No space left
on device".

CC rust-lang#40322
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Jun 28, 2017
…r=GuillaumeGomez

Document possible `io::ErrorKind`s of `fs::open`

Try to make clear that this isn't an API guarantee for now, as we likely
want to refine these errors in the future, e.g. `ENOSPC` "No space left
on device".

CC rust-lang#40322
frewsxcv added a commit to frewsxcv/rust that referenced this issue Jun 29, 2017
…r=GuillaumeGomez

Document possible `io::ErrorKind`s of `fs::open`

Try to make clear that this isn't an API guarantee for now, as we likely
want to refine these errors in the future, e.g. `ENOSPC` "No space left
on device".

CC rust-lang#40322
frewsxcv added a commit to frewsxcv/rust that referenced this issue Jun 29, 2017
…r=GuillaumeGomez

Document possible `io::ErrorKind`s of `fs::open`

Try to make clear that this isn't an API guarantee for now, as we likely
want to refine these errors in the future, e.g. `ENOSPC` "No space left
on device".

CC rust-lang#40322
frewsxcv added a commit to frewsxcv/rust that referenced this issue Jun 30, 2017
…r=GuillaumeGomez

Document possible `io::ErrorKind`s of `fs::open`

Try to make clear that this isn't an API guarantee for now, as we likely
want to refine these errors in the future, e.g. `ENOSPC` "No space left
on device".

CC rust-lang#40322
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jun 30, 2017
…r=GuillaumeGomez

Document possible `io::ErrorKind`s of `fs::open`

Try to make clear that this isn't an API guarantee for now, as we likely
want to refine these errors in the future, e.g. `ENOSPC` "No space left
on device".

CC rust-lang#40322
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jun 30, 2017
…r=GuillaumeGomez

Document possible `io::ErrorKind`s of `fs::open`

Try to make clear that this isn't an API guarantee for now, as we likely
want to refine these errors in the future, e.g. `ENOSPC` "No space left
on device".

CC rust-lang#40322
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jun 30, 2017
…r=GuillaumeGomez

Document possible `io::ErrorKind`s of `fs::open`

Try to make clear that this isn't an API guarantee for now, as we likely
want to refine these errors in the future, e.g. `ENOSPC` "No space left
on device".

CC rust-lang#40322
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jun 30, 2017
…r=GuillaumeGomez

Document possible `io::ErrorKind`s of `fs::open`

Try to make clear that this isn't an API guarantee for now, as we likely
want to refine these errors in the future, e.g. `ENOSPC` "No space left
on device".

CC rust-lang#40322
@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 27, 2017
@steveklabnik steveklabnik added the E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. label May 29, 2018
@steveklabnik
Copy link
Member

Triage: no changes. Vaguely conceptually similar to #24795

@DevQps
Copy link
Contributor

DevQps commented Apr 3, 2019

@steveklabnik I was just browsing through some older T-docs issues, just to get an idea of everything that is still running (and hopefully help a bit in cleaning up stuff!). And then I saw that the io::ErrorKind page already provides some kind of list: https://doc.rust-lang.org/std/io/enum.ErrorKind.html

Maybe we can "solve" this issue by taking one of the following approaches?:

  • At each std::io struct provide one or two lines of documentation that refer to io::ErrorKind for additional details. I think in many cases it's quite clear which errors may be thrown.
  • At each std::io method provide an example which contains error handling with the most commonly thrown error. Would require a lot of work and maintenance and might be a bit too exhaustive? (I guess there are quite a few common errors for networking or file operations.
  • Just leave it as it is. Currently users can reach io::ErrorKind by browsing from io::Result to io::Error to io::ErrorKind. Maybe that is a bit too long but not impossible to find.

I wonder what your thoughts are on this! I might be able to pick this up completely (or partly depending on the amount of work :))

@steveklabnik
Copy link
Member

I was just browsing through some older T-docs issues, just to get an idea of everything that is still running (and hopefully help a bit in cleaning up stuff!)

I've seen that, it's been great 😄

And then I saw that the io::ErrorKind page already provides some kind of list: https://doc.rust-lang.org/std/io/enum.ErrorKind.html

Yes, this is not what the issue is talking about, though :) It seems you know that, but just to be clear.

At each std::io struct provide one or two lines of documentation that refer to io::ErrorKind for additional details. I think in many cases it's quite clear which errors may be thrown.

I don't think this is necessary because

At each std::io method provide an example which contains error handling with the most commonly thrown error. Would require a lot of work and maintenance and might be a bit too exhaustive? (I guess there are quite a few common errors for networking or file operations.

This is what the bug is talking about. And putting it above and below seems like too much to me.

But it's also not about "commonly thrown" errors, it's about enumerating which possible errors can be returned from a particular function. Basically, io::ErrorKind is a superset of every error of every std::io function. Not every function returns every possible error. The idea is to enumerate specifically which errors can be returned from which calls.

That being said, similar to the other issue we had, I'd like @rust-lang/libs to weigh in; is this something you'd want to see? If we document this, it becomes part of the API contract, so such a project would have to work hand in hand with them.

Just leave it as it is. Currently users can reach io::ErrorKind by browsing from io::Result to io::Error to io::ErrorKind. Maybe that is a bit too long but not impossible to find.

Yep, this is also a possibility. It doesn't seem like there's been huge demand for this.

@DevQps
Copy link
Contributor

DevQps commented Apr 3, 2019

@steveklabnik Ahh thanks for the clarification! I think my brain mixed things up a bit while reading other issues. I can imagine that some platforms generate errors which others do not, so it might be tough to create a hard contract for each method but let's wait for the Libs Team to shine their light on this :)

@DevQps
Copy link
Contributor

DevQps commented May 4, 2019

@steveklabnik Sorry for spamming you with this question, but when I try to ping @rust-lang/libs it fails to work. Is this because it only works for members that are part of the Rust organization? I was also wondering!: It's about a month ago now, what are normal durations for pinging a team or member again if no response was received? It would be nice if the libs team can comment on this such that we can pick it up :)

@BurntSushi
Copy link
Member

From quickly skimming this issue, here's my take:

  1. Do we have any concrete use cases that motivate knowing which errors can be returned by I/O functions?
  2. I do kind of feel like presenting a rigid list of possible errors is probably not something we want to do unless there was compelling motivation to do it. In that sense, I like @steveklabnik's earlier suggestion of just listing "common" errors, with no definite language that it's guaranteed to be exhaustive. But in so doing, I kind of wonder how useful that information would be.

cc @rust-lang/libs (since @DevQps's ping didn't work).

@alexcrichton
Copy link
Member

I personally agree with @BurntSushi in that listing common errors sounds good, but we should not strive to provide an exhaustive list.

@DevQps
Copy link
Contributor

DevQps commented May 7, 2019

@alexcrichton Thanks for your response!

I think two more questions remain for me now:

  1. Do we mean by common: "Errors that occurr often" or "Errors that can be thrown by all platforms". Personally I believe we should not list errors that are platform dependent even if they could occur frequently. But listing the most frequently seen errors that are shared by all platforms seems good to me.

  2. Where should we add this information? I guess we should add this at every method inside std::io in an Error section?

I can pick this up if no one else wants to!

@BurntSushi
Copy link
Member

BurntSushi commented May 7, 2019

I think if you just stick to listing the errors that might occur in terms of io::ErrorKind, then that might be good. We aren't necessarily looking to provide rock solid guarantees here, so I think a disclaimer like, "some errors may only occur on specific platforms" would be fine.

I would like to re-iterate my question of the usefulness of these docs though. If we provide a non-exhaustive non-guaranteed list of errors, what purpose does it serve? i.e., What would people use that information for?

Where should we add this information? I guess we should add this at every method inside std::io in an Error section?

I guess it would be any routine that returns an std::io::Result, which is quite a bit more than just the routines in std::io. (e.g., The OP of this issue mentions std::fs::File::open.)

@DevQps
Copy link
Contributor

DevQps commented May 7, 2019

@BurntSushi Personally I wonder how useful it would be as well. I do believe it can be slightly useful for beginners to catch common cases (such as NotFound, or PermissionDenied) but I wonder if it is worth all this extra work. I see the following pro's and con's:

Pro's:

  • We can easily see, when reading a methods description, the most common errors that can be thrown without having to navigate to io::ErrorKind and guess by reading the variant descriptions
    ourselves.

Con's:

  • It will be quite some work to figure out each error that can be thrown by each method, since there are many methods that use io::Result.
  • I can imagine that more methods using io::Result will be added in the future. I think it might tough to make sure that each of them enumerates all the errors like this.
  • The list will not be exhaustive so it's still possible for errors to occur that are not mentioned. Therefore a _ placeholder is always necessary anyway (and recommended as well in case more errors are being added).

I feel a bit 50/50 about this :) I wonder what @alexcrichton and @steveklabnik think about this however. If they still believe we should list the common errors of Result::io, I am willing to do a part of it.

@glandium
Copy link
Contributor

glandium commented May 7, 2019

At the very least, error types that are necessary to handle for correctness should be documented. e.g. #34661

@alexcrichton
Copy link
Member

I wonder if the variants of ErrorKind should be documented themselves and that's it? They could each have a snippet saying "this is where this may come up in practice, for example" with a standard disclaimer about platform compatibility and whatnot. I agree with @BurntSushi that it's probably not worthwhile to go through every single api return io::Result (which is quite a few) and try to document even the common error conditions. Many of them go through to relatively straightforward syscalls where non-Rust documentation can also be consulted.

@steveklabnik
Copy link
Member

Sounds great to me; let's take that approach.

@jonas-schievink jonas-schievink added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Mar 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. P-medium Medium priority T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

10 participants