Skip to content

Commit 94b1de0

Browse files
janbernloehrjanbernloehr
authored
[azfs] Improve error messages (#1565)
1 parent 8559c11 commit 94b1de0

File tree

1 file changed

+75
-51
lines changed

1 file changed

+75
-51
lines changed

‎tensorflow_io/core/filesystems/az/az_filesystem.cc‎

Lines changed: 75 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ void ParseAzBlobPath(const std::string& fname, bool empty_object_ok,
9595
absl::string_view scheme, accountp, objectp;
9696
ParseURI(fname, &scheme, &accountp, &objectp);
9797
if (scheme != "az") {
98-
std::string error_message = absl::StrCat(
98+
const std::string error_message = absl::StrCat(
9999
"Azure Blob Storage path doesn't start with 'az://': ", fname);
100100
TF_SetStatus(status, TF_INTERNAL, error_message.c_str());
101101
return;
@@ -105,8 +105,8 @@ void ParseAzBlobPath(const std::string& fname, bool empty_object_ok,
105105
absl::ConsumeSuffix(&accountp, kAzBlobEndpoint);
106106

107107
if (accountp.empty() || accountp.compare(".") == 0) {
108-
std::string error_message = absl::StrCat(
109-
"Azure Blob Storage path doesn't contain a account name: ", fname);
108+
const std::string error_message = absl::StrCat(
109+
"Azure Blob Storage path doesn't contain an account name: ", fname);
110110
TF_SetStatus(status, TF_INVALID_ARGUMENT, error_message.c_str());
111111
return;
112112
}
@@ -125,8 +125,8 @@ void ParseAzBlobPath(const std::string& fname, bool empty_object_ok,
125125
}
126126

127127
if (!empty_object_ok && object->empty()) {
128-
std::string error_message = absl::StrCat(
129-
"Azure Blob Storage path doesn't contain a object name: ", fname);
128+
const std::string error_message = absl::StrCat(
129+
"Azure Blob Storage path doesn't contain an object name: ", fname);
130130
TF_SetStatus(status, TF_INVALID_ARGUMENT, error_message.c_str());
131131
return;
132132
}
@@ -135,10 +135,14 @@ void ParseAzBlobPath(const std::string& fname, bool empty_object_ok,
135135
return;
136136
}
137137

138+
bool UseDevAccount() {
139+
const auto use_dev_account = std::getenv("TF_AZURE_USE_DEV_STORAGE");
140+
return use_dev_account != nullptr;
141+
}
142+
138143
std::string CreateAzBlobUrl(const std::string& account,
139144
const std::string& container) {
140-
const auto use_dev_account = std::getenv("TF_AZURE_USE_DEV_STORAGE");
141-
if (use_dev_account != nullptr) {
145+
if (UseDevAccount()) {
142146
return "http://127.0.0.1:10000/" + account + "/" + container;
143147
}
144148

@@ -161,8 +165,7 @@ std::string CreateAzBlobUrl(const std::string& account,
161165
std::shared_ptr<Azure::Storage::Blobs::BlobContainerClient>
162166
CreateAzBlobClientWrapper(const std::string& account,
163167
const std::string& container) {
164-
const auto use_dev_account = std::getenv("TF_AZURE_USE_DEV_STORAGE");
165-
if (use_dev_account != nullptr) {
168+
if (UseDevAccount()) {
166169
std::string account_key =
167170
"Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/"
168171
"K1SZFPTOtr/KBHBeksoGMGw==";
@@ -205,9 +208,15 @@ CreateAzBlobClientWrapper(const std::string& account,
205208
return client;
206209
}
207210

211+
std::string StorageExceptionInfo(const Azure::Storage::StorageException& e) {
212+
return absl::StrCat(" (Status: ", e.StatusCode, " ", e.ErrorCode, " ",
213+
e.Message, ")");
214+
}
215+
208216
void ListResources(const std::string& dir, const std::string& delimiter,
209217
Azure::Storage::Blobs::BlobContainerClient& blob_client,
210218
std::vector<std::string>* results, TF_Status* status) {
219+
TF_VLog(1, "ListResources: %s\n", dir.c_str());
211220
if (!results) {
212221
TF_SetStatus(status, TF_INTERNAL, "results cannot be null");
213222
return;
@@ -242,8 +251,8 @@ void ListResources(const std::string& dir, const std::string& delimiter,
242251
});
243252
}
244253
} catch (const Azure::Storage::StorageException& e) {
245-
std::string error_message =
246-
absl::StrCat("Failed to get blobs of ", dir, " (", e.ErrorCode, ")");
254+
const std::string error_message =
255+
absl::StrCat("Failed to get blobs of ", dir, StorageExceptionInfo(e));
247256
TF_SetStatus(status, TF_INTERNAL, error_message.c_str());
248257
return;
249258
}
@@ -260,6 +269,8 @@ class AzBlobRandomAccessFile {
260269
~AzBlobRandomAccessFile() {}
261270
int64_t Read(uint64_t offset, size_t n, char* buffer,
262271
TF_Status* status) const {
272+
TF_VLog(1, "ReadFileFromAz az://%s/%s/%s from %u for n: %u\n",
273+
account_.c_str(), container_.c_str(), object_.c_str(), offset, n);
263274
// If n == 0, then return Status::OK()
264275
// otherwise, if bytes_read < n then return OutofRange
265276
if (n == 0) {
@@ -274,8 +285,9 @@ class AzBlobRandomAccessFile {
274285
auto blob_property = blob_client.GetProperties();
275286
file_size = blob_property.Value.BlobSize;
276287
} catch (const Azure::Storage::StorageException& e) {
277-
std::string error_message =
278-
absl::StrCat("Failed to get properties ", e.ErrorCode);
288+
const std::string error_message =
289+
absl::StrCat("Failed to get properties of az://", account_, "/",
290+
container_, "/", object_, StorageExceptionInfo(e));
279291
TF_SetStatus(status, TF_INTERNAL, error_message.c_str());
280292
return 0;
281293
}
@@ -297,9 +309,9 @@ class AzBlobRandomAccessFile {
297309
blob_client.DownloadTo(reinterpret_cast<uint8_t*>(buffer),
298310
bytes_to_read, download_options);
299311
} catch (const Azure::Storage::StorageException& e) {
300-
std::string error_message = absl::StrCat(
301-
"Failed to get contents of az://", account_, kAzBlobEndpoint, "/",
302-
container_, "/", object_, " (", e.ErrorCode, ")");
312+
const std::string error_message =
313+
absl::StrCat("Failed to get contents of az://", account_, "/",
314+
container_, "/", object_, StorageExceptionInfo(e));
303315
TF_SetStatus(status, TF_INTERNAL, error_message.c_str());
304316
return 0;
305317
}
@@ -373,15 +385,18 @@ class AzBlobWritableFile {
373385
return;
374386
}
375387

388+
TF_VLog(1, "WriteFileToAz: az://%s/%s/%s\n", account_.c_str(),
389+
container_.c_str(), object_.c_str());
390+
376391
auto blob_container_client =
377392
CreateAzBlobClientWrapper(account_, container_);
378393
auto blob_client = blob_container_client->GetBlockBlobClient(object_);
379394
try {
380395
blob_client.UploadFrom(tmp_content_filename_);
381396
} catch (const Azure::Storage::StorageException& e) {
382-
std::string error_message =
397+
const std::string error_message =
383398
absl::StrCat("Failed to upload to az://", account_, "/", container_,
384-
"/", object_, " (", e.ErrorCode, ")");
399+
"/", object_, StorageExceptionInfo(e));
385400
TF_SetStatus(status, TF_INTERNAL, error_message.c_str());
386401
return;
387402
}
@@ -568,6 +583,7 @@ static void NewReadOnlyMemoryRegionFromFile(const TF_Filesystem* filesystem,
568583

569584
static void CreateDir(const TF_Filesystem* filesystem, const char* path,
570585
TF_Status* status) {
586+
TF_VLog(1, "CreateDir %s\n", path);
571587
std::string account, container, object;
572588
ParseAzBlobPath(path, true, &account, &container, &object, status);
573589
if (TF_GetCode(status) != TF_OK) {
@@ -594,6 +610,7 @@ static void RecursivelyCreateDir(const TF_Filesystem* filesystem,
594610

595611
static void DeleteFile(const TF_Filesystem* filesystem, const char* path,
596612
TF_Status* status) {
613+
TF_VLog(1, "DeleteFile %s\n", path);
597614
std::string account, container, object;
598615
ParseAzBlobPath(path, false, &account, &container, &object, status);
599616
if (TF_GetCode(status) != TF_OK) {
@@ -607,8 +624,8 @@ static void DeleteFile(const TF_Filesystem* filesystem, const char* path,
607624
try {
608625
auto response = blob_client.Delete();
609626
} catch (const Azure::Storage::StorageException& e) {
610-
std::string error_message =
611-
absl::StrCat("Failed to delete ", path, "(", e.ErrorCode, ")");
627+
const std::string error_message =
628+
absl::StrCat("Failed to delete ", path, StorageExceptionInfo(e));
612629
TF_SetStatus(status, TF_INTERNAL, error_message.c_str());
613630
return;
614631
}
@@ -617,8 +634,7 @@ static void DeleteFile(const TF_Filesystem* filesystem, const char* path,
617634

618635
static void DeleteDir(const TF_Filesystem* filesystem, const char* path,
619636
TF_Status* status) {
620-
// Doesn't support file delete - call GetChildren (without delimiter) and then
621-
// loop and delete
637+
TF_VLog(1, "DeleteDir %s\n", path);
622638

623639
std::string account, container, object;
624640
ParseAzBlobPath(path, false, &account, &container, &object, status);
@@ -642,8 +658,8 @@ static void DeleteDir(const TF_Filesystem* filesystem, const char* path,
642658
try {
643659
blob_container_client->Delete();
644660
} catch (const Azure::Storage::StorageException& e) {
645-
std::string error_message =
646-
absl::StrCat("Error deleting ", path, " (", e.ErrorCode, ")");
661+
const std::string error_message =
662+
absl::StrCat("Error deleting ", path, StorageExceptionInfo(e));
647663
TF_SetStatus(status, TF_INTERNAL, error_message.c_str());
648664
return;
649665
}
@@ -662,9 +678,9 @@ static void DeleteDir(const TF_Filesystem* filesystem, const char* path,
662678
}
663679
}
664680
} catch (const Azure::Storage::StorageException& e) {
665-
std::string error_message =
666-
absl::StrCat("Failed to list blobs in ", container, "/", object, " (",
667-
e.ErrorCode, ")");
681+
const std::string error_message =
682+
absl::StrCat("Failed to list blobs in az://", account, "/", container,
683+
"/", object, StorageExceptionInfo(e));
668684
TF_SetStatus(status, TF_INTERNAL, error_message.c_str());
669685
return;
670686
}
@@ -674,8 +690,9 @@ static void DeleteDir(const TF_Filesystem* filesystem, const char* path,
674690
try {
675691
child_client.Delete();
676692
} catch (const Azure::Storage::StorageException& e) {
677-
std::string error_message =
678-
absl::StrCat("Failed to delete ", child, " (", e.ErrorCode, ")");
693+
const std::string error_message =
694+
absl::StrCat("Failed to delete az://", account, "/", container, "/",
695+
child, StorageExceptionInfo(e));
679696
TF_SetStatus(status, TF_INTERNAL, error_message.c_str());
680697
return;
681698
}
@@ -695,6 +712,7 @@ static void DeleteRecursively(const TF_Filesystem* filesystem, const char* path,
695712

696713
static void RenameFile(const TF_Filesystem* filesystem, const char* src,
697714
const char* dst, TF_Status* status) {
715+
TF_VLog(1, "RenameFile from: %s to %s\n", src, dst);
698716
std::string src_account, src_container, src_object;
699717
ParseAzBlobPath(src, false, &src_account, &src_container, &src_object,
700718
status);
@@ -710,7 +728,7 @@ static void RenameFile(const TF_Filesystem* filesystem, const char* src,
710728
}
711729

712730
if (src_account != dst_account) {
713-
std::string error_message =
731+
const std::string error_message =
714732
absl::StrCat("Couldn't rename ", src, " to ", dst,
715733
": moving files between accounts is not supported");
716734
TF_SetStatus(status, TF_UNIMPLEMENTED, error_message.c_str());
@@ -731,9 +749,9 @@ static void RenameFile(const TF_Filesystem* filesystem, const char* src,
731749
// Status can be success, pending, aborted or failed
732750
res.PollUntilDone(std::chrono::seconds(1));
733751
} catch (const Azure::Storage::StorageException& e) {
734-
std::string error_message =
735-
absl::StrCat("Failed to start rename from ", src, " to ", dst, " (",
736-
e.ErrorCode, ")");
752+
const std::string error_message =
753+
absl::StrCat("Failed to start rename from ", src, " to ", dst,
754+
StorageExceptionInfo(e));
737755
TF_SetStatus(status, TF_INTERNAL, error_message.c_str());
738756
return;
739757
}
@@ -742,7 +760,7 @@ static void RenameFile(const TF_Filesystem* filesystem, const char* src,
742760
auto copy_status = properties.CopyStatus.Value();
743761

744762
if (copy_status != Azure::Storage::Blobs::Models::CopyStatus::Success) {
745-
std::string error_message =
763+
const std::string error_message =
746764
absl::StrCat("Process of renaming from ", src, " to ", dst,
747765
" resulted in status of ", copy_status.ToString());
748766
TF_SetStatus(status, TF_INTERNAL, error_message.c_str());
@@ -756,8 +774,8 @@ static void RenameFile(const TF_Filesystem* filesystem, const char* src,
756774
try {
757775
src_blob_client.Delete();
758776
} catch (const Azure::Storage::StorageException& e) {
759-
std::string error_message = absl::StrCat(
760-
"Failed to get delete after copy of ", src, " (", e.ErrorCode, ")");
777+
const std::string error_message = absl::StrCat(
778+
"Failed to get delete after copy of ", src, StorageExceptionInfo(e));
761779
TF_SetStatus(status, TF_INTERNAL, error_message.c_str());
762780
return;
763781
}
@@ -767,6 +785,7 @@ static void RenameFile(const TF_Filesystem* filesystem, const char* src,
767785

768786
static void CopyFile(const TF_Filesystem* filesystem, const char* src,
769787
const char* dst, TF_Status* status) {
788+
TF_VLog(1, "CopyFile from: %s to %s\n", src, dst);
770789
// 128KB copy buffer
771790
constexpr size_t kCopyFileBufferSize = 128 * 1024;
772791

@@ -808,6 +827,7 @@ static void CopyFile(const TF_Filesystem* filesystem, const char* src,
808827

809828
static void PathExists(const TF_Filesystem* filesystem, const char* path,
810829
TF_Status* status) {
830+
TF_VLog(1, "PathExists on path: %s\n", path);
811831
std::string account, container, object;
812832
ParseAzBlobPath(path, false, &account, &container, &object, status);
813833
if (TF_GetCode(status) != TF_OK) {
@@ -822,13 +842,13 @@ static void PathExists(const TF_Filesystem* filesystem, const char* path,
822842
} catch (const Azure::Storage::StorageException& e) {
823843
if (e.StatusCode == Azure::Core::Http::HttpStatusCode::NotFound &&
824844
(e.ErrorCode == "BlobNotFound" || e.ErrorCode == "ContainerNotFound")) {
825-
std::string error_message =
845+
const std::string error_message =
826846
absl::StrCat("The specified path ", path, " was not found");
827847
TF_SetStatus(status, TF_NOT_FOUND, error_message.c_str());
828848
return;
829849
} else {
830-
std::string error_message = absl::StrCat("Failed to check if ", path,
831-
" exists (", e.ErrorCode, ")");
850+
const std::string error_message = absl::StrCat(
851+
"Failed to check if ", path, " exists", StorageExceptionInfo(e));
832852
TF_SetStatus(status, TF_NOT_FOUND, error_message.c_str());
833853
return;
834854
}
@@ -838,6 +858,7 @@ static void PathExists(const TF_Filesystem* filesystem, const char* path,
838858

839859
static bool IsDirectory(const TF_Filesystem* filesystem, const char* path,
840860
TF_Status* status) {
861+
TF_VLog(1, "IsDirectory on path: %s\n", path);
841862
// Should check that account and container exist and that fname isn't a file
842863
// Azure storage file system is virtual and is created with path compenents in
843864
// blobs name so no need to check further
@@ -867,13 +888,13 @@ static bool IsDirectory(const TF_Filesystem* filesystem, const char* path,
867888
} catch (const Azure::Storage::StorageException& e) {
868889
if (e.StatusCode == Azure::Core::Http::HttpStatusCode::NotFound &&
869890
(e.ErrorCode == "BlobNotFound" || e.ErrorCode == "ContainerNotFound")) {
870-
std::string error_message =
891+
const std::string error_message =
871892
absl::StrCat("The specified folder ", path, " was not found");
872893
TF_SetStatus(status, TF_NOT_FOUND, error_message.c_str());
873894
return false;
874895
} else {
875-
std::string error_message = absl::StrCat("Failed to check if ", path,
876-
" exists (", e.ErrorCode, ")");
896+
const std::string error_message = absl::StrCat(
897+
"Failed to check if ", path, " exists", StorageExceptionInfo(e));
877898
TF_SetStatus(status, TF_NOT_FOUND, error_message.c_str());
878899
return false;
879900
}
@@ -886,17 +907,16 @@ static bool IsDirectory(const TF_Filesystem* filesystem, const char* path,
886907
try {
887908
auto blob_properties = blob_client.GetProperties();
888909

889-
std::string error_message =
910+
const std::string error_message =
890911
absl::StrCat("The specified folder ", path, " is not a directory");
891912
TF_SetStatus(status, TF_FAILED_PRECONDITION, error_message.c_str());
892913
return false;
893914
} catch (const Azure::Storage::StorageException& e) {
894915
if (e.StatusCode == Azure::Core::Http::HttpStatusCode::NotFound) {
895916
// all good
896917
} else {
897-
std::string error_message =
898-
absl::StrCat("Failed to check if ", path, " exists (", e.StatusCode,
899-
" ", e.Message, " ", e.ErrorCode, ")");
918+
const std::string error_message = absl::StrCat(
919+
"Failed to check if ", path, " exists", StorageExceptionInfo(e));
900920
TF_SetStatus(status, TF_NOT_FOUND, error_message.c_str());
901921
return false;
902922
}
@@ -908,6 +928,8 @@ static bool IsDirectory(const TF_Filesystem* filesystem, const char* path,
908928

909929
static void Stat(const TF_Filesystem* filesystem, const char* path,
910930
TF_FileStatistics* stats, TF_Status* status) {
931+
TF_VLog(1, "Stat on path: %s\n", path);
932+
911933
using namespace std::chrono;
912934

913935
std::string account, container, object;
@@ -939,8 +961,8 @@ static void Stat(const TF_Filesystem* filesystem, const char* path,
939961
auto az_last_modified = blob_property.Value.LastModified.time_since_epoch();
940962
stats->mtime_nsec = duration_cast<nanoseconds>(az_last_modified).count();
941963
} catch (const Azure::Storage::StorageException& e) {
942-
std::string error_message = absl::StrCat("Failed to get file stats for ",
943-
path, " (", e.ErrorCode, ")");
964+
const std::string error_message = absl::StrCat(
965+
"Failed to get file stats for ", path, StorageExceptionInfo(e));
944966
TF_SetStatus(status, TF_NOT_FOUND, error_message.c_str());
945967
return;
946968
}
@@ -950,14 +972,15 @@ static void Stat(const TF_Filesystem* filesystem, const char* path,
950972

951973
static int GetChildren(const TF_Filesystem* filesystem, const char* path,
952974
char*** entries, TF_Status* status) {
975+
TF_VLog(1, "GetChildren on path: %s\n", path);
953976
std::string account, container, object;
954977
ParseAzBlobPath(path, true, &account, &container, &object, status);
955978
if (TF_GetCode(status) != TF_OK) {
956979
return 0;
957980
}
958981

959982
if (container.empty()) {
960-
std::string error_message =
983+
const std::string error_message =
961984
absl::StrCat("Cannot iterate containers in ", path);
962985
TF_SetStatus(status, TF_INTERNAL, error_message.c_str());
963986
return 0;
@@ -1015,6 +1038,7 @@ static int GetChildren(const TF_Filesystem* filesystem, const char* path,
10151038

10161039
static int64_t GetFileSize(const TF_Filesystem* filesystem, const char* path,
10171040
TF_Status* status) {
1041+
TF_VLog(1, "GetFileSize on path: %s\n", path);
10181042
std::string account, container, object;
10191043
ParseAzBlobPath(path, false, &account, &container, &object, status);
10201044
if (TF_GetCode(status) != TF_OK) {
@@ -1029,8 +1053,8 @@ static int64_t GetFileSize(const TF_Filesystem* filesystem, const char* path,
10291053
TF_SetStatus(status, TF_OK, "");
10301054
return blob_property.Value.BlobSize;
10311055
} catch (const Azure::Storage::StorageException& e) {
1032-
std::string error_message = absl::StrCat("Failed to get properties of ",
1033-
path, " (", e.ErrorCode, ")");
1056+
const std::string error_message = absl::StrCat(
1057+
"Failed to get properties of ", path, StorageExceptionInfo(e));
10341058
TF_SetStatus(status, TF_INTERNAL, error_message.c_str());
10351059
return 0;
10361060
}

0 commit comments

Comments
 (0)