Skip to content

Commit

Permalink
Better error message when Table.read cannot open the file (#159)
Browse files Browse the repository at this point in the history
Our current error message on `Table.read` not being able top open/read file is not helpful. Reason is two-fold:
1) the error pretty printing call was missing an argument, so we got error-formetting-error instead of actual error;
2) the actual error wouldn't be helpful anyway -- we ignored the actual error with reading file (that can be caused by wrong permission settings or wrong path), because we expect failure possibility when trying to match proper parser for given file.

Now not being able to read file is a special class of errors that stop the parser-matching routine and allow to give proper diagnostics to the user.
  • Loading branch information
mwu-tow committed Jun 18, 2019
1 parent 783eef9 commit ac9f27e
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 7 deletions.
13 changes: 8 additions & 5 deletions native_libs/src/Core/Common.h
Original file line number Diff line number Diff line change
Expand Up @@ -167,12 +167,15 @@ std::vector<T> iotaVector(size_t N, T from = T{})
return ret;
}

#define THROW_OBJ(EXC_TO_THROW) do { \
std::cerr << __FILE__ << " " << __LINE__ << " " << EXC_TO_THROW.what() << std::endl; \
throw EXC_TO_THROW; \
} while(0)

#define THROW(message, ...) do { \
auto msg_ = fmt::format(message, ##__VA_ARGS__); \
std::cerr << __FILE__ << " " << __LINE__ << " " << msg_ << std::endl; \
std::runtime_error e_to_throw{fmt::format(message, ##__VA_ARGS__)}; \
throw e_to_throw; \
#define THROW(message, ...) do { \
auto msg_ = fmt::format(message, ##__VA_ARGS__); \
std::runtime_error e_to_throw{fmt::format(message, ##__VA_ARGS__)}; \
THROW_OBJ(e_to_throw); \
} while(0)

namespace detail
Expand Down
15 changes: 13 additions & 2 deletions native_libs/src/IO/IO.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ std::shared_ptr<arrow::Table> readTableFromFile(std::string_view filepath)
if(auto table = handler->tryReading(filepath))
return table;

THROW("Failed to load file {}: it doesn't parse with default settings as any of the supported formats");
THROW("Failed to load file {}: it doesn't parse with default settings as any of the supported formats", filepath);
}

void writeTableToFile(std::string_view filepath, const arrow::Table &table)
Expand Down Expand Up @@ -132,7 +132,13 @@ std::ifstream openFileToRead(std::string_view filepath)
#endif

if(!in)
THROW("Cannot open the file to read: {}", filepath);
{
// Do not bother catching std::ifstream::failure and checking its supposedly inherited `std::system_error::code`.
// libstdc++ does not implement it and even on platforms where it works, the error message is not helpful at all.
// If we really want to give proper diagnostics for filesystem errors, we would need to get platform-specific.
CannotOpenToRead cannotRead{ filepath };
THROW_OBJ(cannotRead);
}

return in;
}
Expand Down Expand Up @@ -183,6 +189,11 @@ std::shared_ptr<arrow::Table> TableFileHandler::tryReading(std::string_view file

return read(filePath);
}
catch(CannotOpenToRead &)
{
// this is a serious one -- there is no sense in trying reading something that cannot be read anyway
throw;
}
catch(...)
{
return nullptr;
Expand Down
7 changes: 7 additions & 0 deletions native_libs/src/IO/IO.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ namespace arrow
class DataType;
}

struct CannotOpenToRead : std::runtime_error
{
CannotOpenToRead(std::string_view path)
: std::runtime_error(fmt::format("Cannot open file to read: {}", path))
{}
};

struct TakeFirstRowAsHeaders {};
struct GenerateColumnNames {};

Expand Down
6 changes: 6 additions & 0 deletions native_libs/test/Tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1162,6 +1162,12 @@ BOOST_AUTO_TEST_CASE(SliceBoundsChecking)
BOOST_CHECK_NO_THROW(slice(column, 0, 5));
}

BOOST_AUTO_TEST_CASE(ReadMissingCsvFile)
{
const auto pathUnlikelyToExist = "data/gbhfudbfiugbiu.csv";
BOOST_CHECK_THROW(readTableFromFile(pathUnlikelyToExist), CannotOpenToRead);
}

BOOST_AUTO_TEST_CASE(ReadCsvFileWithBOM)
{
const auto t = FormatCSV{}.read("data/fileWithBOM.csv");
Expand Down

0 comments on commit ac9f27e

Please sign in to comment.