Save error on the context before returning error code in vfs_open (#4347

) Save error on the context before returning error code in `tiledb_vfs_open`. --- TYPE: BUG DESC: Save error on the context before returning error code in vfs_open (cherry picked from commit ea25897)
TileDB-Inc · Sep 12, 2023 · 680e59c · 680e59c
1 parent 323c99c
commit 680e59c
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 8 deletions.
diff --git a/test/src/unit-capi-vfs.cc b/test/src/unit-capi-vfs.cc
@@ -290,7 +290,7 @@ void VFSFx::check_vfs(const std::string& path) {
   rc = tiledb_vfs_is_file(ctx_, vfs_, foo_file.c_str(), &is_file);
   REQUIRE(rc == TILEDB_OK);
   REQUIRE(!is_file);
-  tiledb_vfs_fh_t* fh;
+  tiledb_vfs_fh_t* fh = nullptr;
   rc = tiledb_vfs_open(ctx_, vfs_, foo_file.c_str(), TILEDB_VFS_READ, &fh);
   REQUIRE(rc == TILEDB_ERR);
   REQUIRE(fh == nullptr);

diff --git a/tiledb/api/c_api/vfs/test/unit_capi_vfs.cc b/tiledb/api/c_api/vfs/test/unit_capi_vfs.cc
@@ -658,3 +658,20 @@ TEST_CASE("C API: tiledb_vfs_fh_is_closed argument validation", "[capi][vfs]") {
 TEST_CASE("C API: tiledb_vfs_fh_free argument validation", "[capi][vfs]") {
   REQUIRE_NOTHROW(tiledb_vfs_fh_free(nullptr));
 }
+
+TEST_CASE(
+    "C API: tiledb_vfs_open reports error when open fails", "[capi][vfs]") {
+  ordinary_vfs x;
+  tiledb_vfs_fh_handle_t* vfs_fh;
+
+  tiledb_error_t* error = nullptr;
+
+  capi_return_t rc = tiledb_vfs_open(
+      x.ctx, x.vfs, "doesnotexistfile", TILEDB_VFS_READ, &vfs_fh);
+  REQUIRE(tiledb_status(rc) == TILEDB_ERR);
+
+  rc = tiledb_ctx_get_last_error(x.ctx, &error);
+  REQUIRE(tiledb_status(rc) == TILEDB_OK);
+
+  CHECK(error != nullptr);
+}
diff --git a/tiledb/api/c_api/vfs/vfs_api.cc b/tiledb/api/c_api/vfs/vfs_api.cc
@@ -229,14 +229,9 @@ capi_return_t tiledb_vfs_open(
     throw CAPIStatusException(std::string("Invalid TileDB object: ") + "uri");
   }
   auto vfs_mode = static_cast<tiledb::sm::VFSMode>(mode);
-  *fh = tiledb_vfs_fh_t::make_handle(fh_uri, vfs->vfs(), vfs_mode);
 
-  // Open VFS file
-  auto st{(*fh)->open()};
-  if (!st.ok()) {
-    tiledb_vfs_fh_t::break_handle(*fh);
-    return TILEDB_ERR;
-  }
+  // Throws if opening the uri is unsuccessful
+  *fh = tiledb_vfs_fh_t::make_handle(fh_uri, vfs->vfs(), vfs_mode);
 
   return TILEDB_OK;
 }

diff --git a/tiledb/api/c_api/vfs/vfs_api_internal.h b/tiledb/api/c_api/vfs/vfs_api_internal.h
@@ -165,6 +165,7 @@ struct tiledb_vfs_fh_handle_t
       tiledb::sm::VFS* vfs,
       tiledb::sm::VFSMode mode)
       : vfs_fh_{uri, vfs, mode} {
+    throw_if_not_ok(vfs_fh_.open());
   }
 
   Status open() {