Skip to content

Commit e4230db

Browse files
teo-tsirpanisihnorton
authored andcommitted
Improve error handling on S3 HEAD requests. (TileDB-Inc#5380)
[SC-59519](https://app.shortcut.com/tiledb-inc/story/59519/s3-is-object-masks-failures) [SC-59575](https://app.shortcut.com/tiledb-inc/story/59575/improve-error-message-when-headobject-fails-with-status-301) * In `S3::is_object` we return that the object does not exist if the `HeadObject` API call fails for any reason (like unauthorized or wrong region). This PR changes to return `false` only if `HeadObject` fails with a 404 status, and return the failure to the user in other cases. * Azure and GCS already do that. * In S3 operations that perform `HeadObject` (`is_object` and `object_size`), if the operation returns a 301 (Permanent Redirect) status, we add an explanatory note to the error message suggesting that the configured region might be incorrect. This will be helpful because while most operations return a detailed error message, `HeadObject` cannot have a response body. --- TYPE: BUG DESC: Fixed `tiledb_vfs_is_file` masking failures on S3 by returning false. --- TYPE: IMPROVEMENT DESC: Added additional context in the error messages of operations that likely failed due to region mismatch.
1 parent 71953f9 commit e4230db

File tree

1 file changed

+40
-4
lines changed
  • tiledb/sm/filesystem

1 file changed

+40
-4
lines changed

tiledb/sm/filesystem/s3.cc

+40-4
Original file line numberDiff line numberDiff line change
@@ -910,8 +910,31 @@ Status S3::is_object(
910910
if (request_payer_ != Aws::S3::Model::RequestPayer::NOT_SET)
911911
head_object_request.SetRequestPayer(request_payer_);
912912
auto head_object_outcome = client_->HeadObject(head_object_request);
913-
*exists = head_object_outcome.IsSuccess();
914913

914+
if (!head_object_outcome.IsSuccess()) {
915+
if (head_object_outcome.GetError().GetResponseCode() ==
916+
Aws::Http::HttpResponseCode::NOT_FOUND) {
917+
*exists = false;
918+
return Status::Ok();
919+
}
920+
921+
// Because this is a HEAD request, its response will not contain a detailed
922+
// message. Try to provide more information to the user depending on the
923+
// status code.
924+
std::string additional_context;
925+
if (head_object_outcome.GetError().GetResponseCode() ==
926+
Aws::Http::HttpResponseCode::MOVED_PERMANENTLY) {
927+
additional_context =
928+
" The bucket might be located in another region; you can set it with "
929+
"the 'vfs.s3.region' option.";
930+
}
931+
return LOG_STATUS(Status_S3Error(
932+
"Failed to check for existence of object s3://" + bucket_name + "/" +
933+
object_key + outcome_error_message(head_object_outcome) +
934+
additional_context));
935+
}
936+
937+
*exists = true;
915938
return Status::Ok();
916939
}
917940

@@ -1029,10 +1052,23 @@ Status S3::object_size(const URI& uri, uint64_t* nbytes) const {
10291052
head_object_request.SetRequestPayer(request_payer_);
10301053
auto head_object_outcome = client_->HeadObject(head_object_request);
10311054

1032-
if (!head_object_outcome.IsSuccess())
1055+
if (!head_object_outcome.IsSuccess()) {
1056+
// Because this is a HEAD request, its response will not contain a detailed
1057+
// message. Try to provide more information to the user depending on the
1058+
// status code.
1059+
std::string additional_context;
1060+
if (head_object_outcome.GetError().GetResponseCode() ==
1061+
Aws::Http::HttpResponseCode::MOVED_PERMANENTLY) {
1062+
additional_context =
1063+
" The bucket might be located in another region; you can set it with "
1064+
"the 'vfs.s3.region' option.";
1065+
}
10331066
return LOG_STATUS(Status_S3Error(
1034-
"Cannot retrieve S3 object size; Error while listing file " +
1035-
uri.to_string() + outcome_error_message(head_object_outcome)));
1067+
std::string(
1068+
"Cannot retrieve S3 object size; Error while listing file s3://") +
1069+
uri.to_string() + outcome_error_message(head_object_outcome) +
1070+
additional_context));
1071+
}
10361072
*nbytes =
10371073
static_cast<uint64_t>(head_object_outcome.GetResult().GetContentLength());
10381074

0 commit comments

Comments
 (0)