Custom error messages for IO with nonexistent files #14662
Conversation
vuule
added
non-breaking
…vuule/cudf into impr-failed-file-open-error-msg
vuule
changed the title
cpp/src/io/utilities/data_sink.cpp
Outdated
| CUDF_FAIL("Cannot open output file; directory does not exist"); | ||
| } | ||
| CUDF_FAIL("Cannot open output file"); | ||
| } |
There was a problem hiding this comment.
would it help to make this code also similar to cpp/src/io/utilities/file_io_utilities.cpp ? That seems more detailed.
There was a problem hiding this comment.
added errno. I originally excluded it because I didn't know if ofstream::open sets it.
There was a problem hiding this comment.
Should we move this into file_io_utilities? As it should contain all file IO.
There was a problem hiding this comment.
TIL std::strerror.
I had to look up where the allocation happens for the string returned by std::strerror(). I'm not convinced it is threadsafe, given that it's allowed to reuse the same allocated space to return strings across threads.
Should we consider switching this to std::strerror_s()?
Edit: Source.
There was a problem hiding this comment.
great catch! I'll look into strerror_s.
There was a problem hiding this comment.
Should we move this into file_io_utilities?
I don't think file_sink belongs in utilities, but I'll try to reuse this error checking and place the common code in file_io_utilities.
There was a problem hiding this comment.
I should have just put a mutex in here. strerror_s portability is horrendous.
Our compiler does not support it, so I have to use strerror_r. Aaand, this one is problematic as well, see https://www.club.cc.cmu.edu/~cmccabe/blog_strerror.html
…impr-failed-file-open-error-msg
…impr-failed-file-open-error-msg
|
/merge |
Description
Closes #12311; closes #9564
We return a somewhat cryptic error when opening a file that does not exist: "Cannot query file size".
With this change, we report whether the file exists, or, if the file does exist, what the errno value is after
open.Also added a check for the output files' directory in
file_sink. This check is now also included infile_wrapper, just in case initialization order changes at some point.Now we should always correctly report missing output file directory and missing input files.
Checklist