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

The FileSystemInfo.Exists anomaly; File.Exists() has no use #24937

Closed
jhudsoncedaron opened this issue Feb 6, 2018 · 10 comments
Closed

The FileSystemInfo.Exists anomaly; File.Exists() has no use #24937

jhudsoncedaron opened this issue Feb 6, 2018 · 10 comments
Milestone

Comments

@jhudsoncedaron
Copy link

This bug is mostly in the documentation.

File.Exists() returns false if an I/O error or permission error causes it to be impossible to determine if the file exists or not. To get the error, call LastWriteTimeUTC() and throw away the result. If LastWriteTimeUTC() returns, the file doesn't exist. If it throws, you don't know if the file exists or not and the exception is the reason you don't know.

This behavior is not documented. The apparent behavior from the documentation is FileInfo.Exists throws if an I/O error or permission error happens.

Not writing the extra call to check if FileSystemInfo.Exists() is reliable on returning false causes corner case bugs to exist in the code that cause incorrect behavior on transient network drops rather than detect and retry.

Due to this behavior, no correct code can ever call File.Exists() or Directory.Exists() even assuming static filesystems; should be tagged obsolete.

@danmoseley
Copy link
Member

File.Exists does document that failure and "does not exist" cannot be distinguished:
https://docs.microsoft.com/en-us/dotnet/api/system.io.file.exists?view=netcore-2.0
"The Exists method returns false if any error occurs while trying to determine if the specified file exists. This can occur in situations that raise exceptions such as passing a file name with invalid characters or too many characters, a failing or missing disk, or if the caller does not have permission to read the file."

However FileSystemInfo.Exists does not document this:
https://docs.microsoft.com/en-us/dotnet/api/system.io.filesysteminfo.exists?view=netcore-2.0

It seems to me it should @JeremyKuhne do you agree?

@jhudsoncedaron does that address your concern?

@jhudsoncedaron
Copy link
Author

@danmosemsft: Mostly. File.Exists's documentation should have "The Exists method returns false if any error occurs while trying to determine if the specified file exists." in the returns section instead of the long enumeration of only some of the cases where it returns false. Most people would otherwise go "I not passing null. I don't care about invalid path. I don't care about no permission to directory. Therefore I'm fine." and never reach the remarks section where I/O error is finally mentioned.

In typing this up the first time I only looked at FileSystemInfo.Exists's documentation where it's nowhere to be found.

@danmoseley
Copy link
Member

danmoseley commented Feb 8, 2018

@jhudsoncedaron makes sense. Do you want to open an issue or PR in the docs repo and close this?

@jhudsoncedaron
Copy link
Author

Are the XML comments in the corefx repo or the docs repo?

@danmoseley
Copy link
Member

I am not sure whether the XML comments in the code end up in the docs or the docs repo is separate. @karelz do you know?

@karelz
Copy link
Member

karelz commented Feb 8, 2018

Changes to XML comments are not reflected in any docs. If you want to change docs, please change them on dotnet/docs repo.

@JeremyKuhne
Copy link
Member

Yes, the docs should be updated to be very clear. The results are really "yes" and "we don't know". "CouldReach" is probably a better term than Exists. You're welcome to make an issue/PR in the docs repository, let me know if you aren't interested and I'll create one. We should also call out the common error where people assume Exists == true means that the file will still be there when they try and access it.

@jhudsoncedaron
Copy link
Author

I'll make the issue soon enough. The only hope I had of a PR was if the docs came from the XML comments, which they don't.

@jhudsoncedaron
Copy link
Author

@danmoseley
Copy link
Member

Thanks @jhudsoncedaron !

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 2.1.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants