Note that for reliably good error messages, you don’t just want to report the file name on errors from File::open(), but also on errors from any of the following File::read()s and any of the parsing of the data read from the file. The reliable way to get this is to attach error context around the entire function (or, someday, try block) responsible for reading the file, which isn’t something std::fs can handle.
So, I’d recommend taking the other functions in std::fs as examples of where reporting a path by default would be useful. (Perhaps particularly std::fs::create_dir_all()?)
Not only do I like this approach, it's more backwards-compatible than I expected, because it seems that even if the error case of a Result borrows from the function's arguments, it can still be converted Into other error types using ? – the ? does the conversion while the arguments are still in scope. So this means that a viable approach might be for the initially created error type to borrow the path argument, but to allow it to be converted Into either a boxed error type with a copy of the path, or a less detailed error type that discards the path (or even left as-is in the case where the path is 'static or was borrowed from the caller). This is essentially your trait-based approach, but with the trait being Into (typically defined via From).
Unfortunately, despite being backwards-compatible with typical coding styles, it isn't backwards-compatible with all current code (because the error type of File::open is defined as Result<T, io::Error> rather than Result<T, impl Into<io::Error>> – the latter has a lifetime in the 2024 edition, the former doesn't). So this is more something that would be used in a std-like crate or a new version of std itself, rather than something that could be easily retrofitted into current std.
As @kpreid pointed out, open is the easy case. You might not have a file name later on, only a file descriptor or handle. And depending on the OS that might not be possible to get back. At least on Linux I think you could inspect /proc/self/fd (assuming procfs is mounted, not guaranteed in containers or early boot), but I don't believe there is any way to get back the file name on POSIX in general (I don't know if there are OS specific ways on the BSDs etc).
I would really not like if opening a file allocated a string to store a copy of the file name in the Rust code. One of my use cases is a file system integrity checker (comparing installed files to the Linux package manager database) and I spent a great deal of effort reducing allocations there as it was one of the major overheads (especially on musl).
Returning the path from a call on error is a significantly different case than saving the path on success and keeping it around for the entire lifetime of the file handle.
Preserving path only on error in calls that got it as an argument shouldn't impact most programs, since failures are typically the unusual case, errors are unlikely to be long-lived, and applications likely want to keep and report the path anyway.
OTOH proactively preserving the path on success can be undesirable, because that adds work to the common case, where it might add up to having noticeable overhead, and in majority of cases later file I/O won't fail, so that extra work is wasted.
So let's not let perfect be enemy of good, and skip having a path for every read/write call. Handling it for just open will already make a massive difference in the frequent cases of Not Found and Permission Denied. The less common cases of actual I/O dying in the middle of a file will keep being vague (arguably when a disk gets disconnected, has a hardware failure, or is full, it's not a path-specific problem but a disk-wide problem, so having a path then is not as critical to fixing the issue).
A static we server these days will see lots of 404s though. My nginx instance sees around a third of its traffic from bots probing "are you a vulnerable WordPress/phpmyadmin?". And many of us don't want to hide behind cloudflare or similar: a) we don't need it b) I don't want someone else to terminate TLS c) Cloudflare is US based.
So the error case can be quite common, and we don't want to make ir easy to use it as a DOS vector. As such I think there needs to be the ability to opt out of such allocations for use cases such as Web servers.
A single path allocation per-request is nothing for a web server. And you’re already doing it, translating the rust Path into something that can be passed to libc requires an allocation anyway. If the handling is smart that allocation can just be kept alive longer rather than adding another allocation for it.
In servers that aren't sitting idle, 404s from bots looking for specific URLs are a small fraction of the traffic.
Rust servers typically already allocate on the hot path (spawning a Future). Every format! allocates. Bytes allocates too.
Allocators can do 100 millions of allocations per second. Allocations are faster than syscalls required to actually open the file.
DoS is a completely unrealistic scenario. You won't be able to defend yourself from a 100 million-requests-per-second DoS attack naively by not rejecting any traffic and serving it all thanks to one microoptimization in one case. At such scale your hosting provider will be in trouble themselves, and will most likely stop routing traffic to your IP address completely. DoS attackers targeting layer 7 don't waste time on 404s, but amplify the attack by hitting expensive URLs that do real work, e.g. website search or login forms, which are a few orders of magnitude more expensive than a 404.
I proposed OpenOptions flag to disable storing the path for those who really really care to microptimize this. But such benchmark-oriented servers would likely already drop below std to use syscalls like sendfile.
std is the lowest common denominator. It doesn't have to be maximally optimized for edgiest edge cases.
FWIW: I'm in full agreement that the higher level helpers in fs should allocate context in the error case if that's something that they have access to. In my mind, that's any call that may call more than one syscall in success cases currently.
However, for lower level functions like File::open, that can reasonably be expected to be equivalent to OpenOptions::new().read(true).open(path), which can be reasonably expected to be a single syscall. Many devs do expect to be able to closely control the behavior of their program even when using the platform agnostic APIs instead of platform specific ones, even if only given an implicit contract of no extra "fluff".
Also, any application that cares about reducing allocation to the point that an extra allocation on a file open error is noticable, should probably already be using APIs which take some kind of &OsZStr instead of &Path so they visibly control the cost of conversion to the OS nul-terminated format.[1]
My current leaning is that OpenOptions should grow a new enhanced_errors flag which should be used by whichever fs functions have access to data with which to enhance the data already (thus do not need to save or retrieve it), and the File constructor methods should set it. The flag will remain false by default for direct users of OpenOptions.
The case of File::read could store a file handle in the error and use that to fetch filename at when the error is rendered (if it ever) is, theoretically. Though there's extra risk now that leaking or otherwise hanging onto the error would keep the file open where it didn't before, so… probably not a great idea.
Time machine speculation, my preferred solution would include io::Result<T> being an alias for Result<T, io::Error<'static>>, and functions like fs::read(path: &impl AsRef<Path>) -> Result<String, io::Error<'_>> where the error info can borrow the input path and provide into_owned()/into() to the 'static form of the error. But that requires a lot of hypothetical type system functionality which rustc doesn't have today and is unlikely to get any time soon.
Indeed it feels a little bit awkward to not be able to handle the conversion via the type system outside of std, even though I understand the real benefits of using UTF-8 (sometimes WTF-8) everywhere except at the exact FFI boundary. It's just on my mind when I'm working off and on on a library binding that's aiming to be truely zero-overhead and thus uses &CStr8 for passing in file paths (that are generally expected to be known locations relative to the running directory and accessed potentially many times). ↩︎
We keep discussing storing the filename somewhere (makes sense), but what if that isn't necessary?
Any file IO has to store the file descriptor (c_int, on unix). We could store this in the error instead of the filename and look up /proc/self/fd/<fd>. Problems with that approach:
Effectively an unchecked weak reference (unless we can make it a real weak reference checked at runtime)
The file might have been closed since and this symlink no longer exists
A new file with the same fd might have been opened since, resulting in a wrong filename
After a rename it might return the new name and not what was initially given to open
These all sound bad, but the worst that can happen is returning no or a wrong filename in the error message. This may cause issues, yes, but it could be fine if documented/named accordingly. The biggest issue would be when the file is closed before the error is debug printed (or the respective method is called), which is when it might return bad data.
let mut f = File::open("foo");
if let Err(e) = f.read(...) {
dbg!(&e); // Might try to look up the filename
e.best_effort_filename(); // returns Option<String>
}
Whether that is a good Idea: I don't know, but it would move basically all cost to the debug formatting. It likely won't work on all operating systems.
It can't work for open failures, because then you don't get any fd at all.
It might technically be possible for read errors, but I'm concerned that it'd be a high risk implementation. Because it could get a different path than what has been opened (when it works at all), it would have to be clearly communicated to users, and wording for that is going to be a tricky thing to bikeshed. It's also not universally supported and has platform-specific limitations.
I think keeping paths only in functions that receive a path is an 80/20 solution. It's reliable, predictable, cross-platform, and won't surprise anyone by performing additional filesystem access (important for sandboxing!) Just keeping the given path handles the most common (and most frustrating) case of a file not found.
/proc/self/fd is Linux-specific, and is not guaranteed to be available even on Linux (it isn't mandatory to mount procfs, and isn't mandatory to mount it at /proc – in practice most Linux distributions do, but /proc might be missing from a container and won't be available during early boot). But a solution here should ideally work on all OSes, not just Linux.