Consider using fs-err to improve I/O error messages
#270
Labels
enhancement
New feature or request
Comments
This was referenced Sep 9, 2024
|
Filed andrewhickman/fs-err#59. |
This was fixed in andrewhickman/fs-err#60 which was released in v3.0.0 of |
edmorley
added a commit
that referenced
this issue
Apr 28, 2025
This improves the error messages shown for file related I/O errors, by: - ensuring that the path is always included - adding additional suggestions to the error based on the context (which is now known, since specialised types are used for each of the common scenarios like checking if a file exists or reading a file to a string) We're able to make the errors richer than those provided by `fs-err`, since we know the context in which the errors have occurred. For example, in the error message for `ReadOptionalFileError`s we know not to say to check for a missing file or that the file is a directory, since `read_optional_file()` explicitly excludes those cases by design. If we used `io::Error` (or `fs-err`'s equivalent) we wouldn't have to rely on the user not using the wrong logging function for the wrong I/O error case, rather than relying on the type system to do that for us. These changes leave `log_io_error()` as being used only for the `Command` I/O cases. A later PR will switch adjust those usages too (but this PR is already large enough). Closes #270. GUS-W-12650236.
|
Closing in favour of the approach in #355. |
edmorley
added a commit
that referenced
this issue
Apr 29, 2025
This improves the error messages shown for file related I/O errors, by: - ensuring that the path is always included - adding additional suggestions to the error based on the context (which is now known, since specialised types are used for each of the common scenarios like checking if a file exists or reading a file to a string) We're able to make the errors richer than those provided by `fs-err`, since we know the context in which the errors have occurred. For example, in the error message for `ReadOptionalFileError`s we know not to say to check for a missing file or that the file is a directory, since `read_optional_file()` explicitly excludes those cases by design. If we used `io::Error` (or `fs-err`'s equivalent) we would have to rely on the user not using the wrong logging function for the wrong I/O error case, rather than relying on the type system to do that for us. These changes leave `log_io_error()` as being used only for the `Command` I/O cases. A later PR will switch adjust those usages too (but this PR is already large enough). Closes #270. GUS-W-12650236.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
The
fs-errcrate aims to improve error messages for I/O errors by eg including the paths of the file/... that triggered the error:https://crates.io/crates/fs-err
It's supposed to be a drop-in replacement, however, testing locally I found that it made the error message worse in some cases.
For example, before switching to fs-err, the output from the
runtime_txt_io_errorintegration test (whose fixture intentionally hasruntime.txtthat contains invalid unicode) was:And after:
With the error message temporarily changed to print the debug output, we can more clearly see the issue:
ie: fs-err wraps the underlying cause of the io::Error, and that cause is not included in the display representation of the top level error. Instead, one has to manually check for and print
error.source(). This is apparently by design (which I get, but it still means more work to use fs-err, and it not quite being the drop-in replacement it promises):andrewhickman/fs-err#51
It may still be worth using
fs-err- however, if we adopt it then further buildpack changes will be required to maintain all error message content (such as having thelog_io_errorfunction print a "caused by" line ifferror.sources()isSome).The text was updated successfully, but these errors were encountered: