Skip to content

Commit

Permalink
Show reasons a POSIX file cannot be read (#5432)
Browse files Browse the repository at this point in the history
Twice in the last few weeks, doing operational work, I've seen the error
message

```
tiledb.cc.TileDBError: [TileDB::Array] Error: ArrayDirectory: IO Error:
Cannot read from file; Read exceeds file size
```

This error happens (rightly) when `offset + nbytes > file_size`. In
order to be helpful, the message should say what those numbers are. If
`offset` is 10 and `nbytes` is 5 but `file_size` is 0, that's a data
problem to debug; if `nbytes` is in the billions, that's more likely a
software problem to debug.

In any case, the error message should be helpful. As-is, not only does
the message hide useful information, it doesn't even show the URI in
question. This forces whoever's doing debug work to rebuild core in
debug mode, re-run the repro in the debugger, and print things out in
the debugger. It's a more effective use of time for the message to show
that debug information.

See also [[sc-62426]](https://app.shortcut.com/tiledb-inc/story/62426)
for Windows CI issue (@teo-tsirpanis)

---
TYPE: IMPROVEMENT
DESC: Show file size, offset, nbytes, and URI in file-read error message

---------

Co-authored-by: Theodore Tsirpanis <[email protected]>
  • Loading branch information
johnkerl and teo-tsirpanis authored Jan 24, 2025
1 parent 40726b7 commit 02808ba
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 9 deletions.
8 changes: 6 additions & 2 deletions tiledb/sm/filesystem/mem_filesystem.cc
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,12 @@ class MemFilesystem::File : public MemFilesystem::FSNode {
assert(buffer);

if (offset + nbytes > size_)
return LOG_STATUS(
Status_MemFSError("Cannot read from file; Read exceeds file size"));
return LOG_STATUS(Status_MemFSError(fmt::format(
"Cannot read from file; Read exceeds file size: offset {} nbytes {} "
"size_ {}",
offset,
nbytes,
size_)));

memcpy(buffer, (char*)data_ + offset, nbytes);
return Status::Ok();
Expand Down
11 changes: 9 additions & 2 deletions tiledb/sm/filesystem/posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,15 @@ void Posix::read(
auto path = uri.to_path();
uint64_t file_size;
this->file_size(URI(path), &file_size);
if (offset + nbytes > file_size)
throw IOError("Cannot read from file; Read exceeds file size");
if (offset + nbytes > file_size) {
throw IOError(fmt::format(
"Cannot read from file; Read exceeds file size: offset {}, nbytes {}, "
"file_size {}, URI {}",
offset,
nbytes,
file_size,
uri.to_path()));
}

// Open file
int fd = open(path.c_str(), O_RDONLY);
Expand Down
24 changes: 19 additions & 5 deletions tiledb/sm/filesystem/win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@
#include "uri.h"
#include "win.h"

#include <fmt/format.h>

using namespace tiledb::common;
using tiledb::common::filesystem::directory_entry;

Expand Down Expand Up @@ -462,11 +464,23 @@ Status Win::read(
0) {
auto gle = GetLastError();
CloseHandle(file_h);
return LOG_STATUS(Status_IOError(
"Cannot read from file '" + path + "'; File read error " +
(gle != 0 ? get_last_error_msg(gle, "ReadFile") :
"num_bytes_read " + std::to_string(num_bytes_read) +
" != nbyes " + std::to_string(nbytes))));

std::string err_msg;
if (gle != 0) {
err_msg = get_last_error_msg(gle, "ReadFile");
} else {
err_msg = std::string("num_bytes_read ") +
std::to_string(num_bytes_read) + " != nbytes " +
std::to_string(nbytes);
}

return LOG_STATUS(Status_IOError(fmt::format(
"Cannot read from file '{}'; File read error '{}' offset {} nbytes "
"{}",
path,
err_msg,
offset,
nbytes)));
}
byte_buffer += num_bytes_read;
offset += num_bytes_read;
Expand Down

0 comments on commit 02808ba

Please sign in to comment.