Azure / azure-sdk-for-cpp

This repository is for active development of the Azure SDK for C++. For consumers of the SDK we recommend visiting our versioned developer docs at https://azure.github.io/azure-sdk-for-cpp.
MIT License
179 stars 128 forks source link

From user study: No useful error message shows up when creating a block blob client to download a blob with a blob name that doesn't exist #4689

Open ahsonkhan opened 1 year ago

ahsonkhan commented 1 year ago

The expectation is that whenever there is an error in the developer code (for example something like a request failed exception), there is some information printed to the console that can help provide a clue on what went wrong.

Following the instructions from here, the user ends up with the following code: https://learn.microsoft.com/en-us/azure/storage/blobs/quickstart-blobs-c-plus-plus?tabs=environment-variable-windows#upload-blobs-to-a-container

In the study, there is no blob in the container with the name blob.txt, instead, it is called blob1.txt. When the user runs their application (which has no compiler error), it doesn't show any information of why it failed.

const string storageConnectionString = getenv("AZURE_STORAGE_CONNECTION_STRING");
const string containerName = "blobcontainer";

// TASK 1: Create a Storage Blob Container Client
cout << "Creating Storage Blob Container Client" << endl;

// Initialize a new instance of BlobContainerClient
BlobContainerClient containerClient
    = BlobContainerClient::CreateFromConnectionString(storageConnectionString, containerName);

// Create the container. This will do nothing if the container already exists.
std::cout << "Creating container: " << containerName << std::endl;
containerClient.CreateIfNotExists();

// TASK 2: List the Blobs in the Storage Container
cout << "Listing blobs..." << endl;
auto listBlobsResponse = containerClient.ListBlobs();
for (auto blobItem : listBlobsResponse.Blobs)
{
    std::cout << "Blob name: " << blobItem.Name << std::endl;
}

// TASK 3: Download a Blob from the Storage Container
cout << "Downloading Blob" << endl;

// ISSUE: There is no blob in the container with this name
std::string blobName = "blob.txt";

// ISSUE: Should this fail with a visible error message for a clue of what went wrong? Maybe.
BlockBlobClient blobClient = containerClient.GetBlockBlobClient(blobName);
auto properties = blobClient.GetProperties().Value;
std::vector<uint8_t> downloadedBlob(properties.BlobSize);

// ISSUE: This should certainly fail with an appropriate error message, which highlights something
// like an RequestFailedException outputted to the console.
blobClient.DownloadTo(downloadedBlob.data(), downloadedBlob.size());
std::cout << "Downloaded blob contents: " << std::string(downloadedBlob.begin(), downloadedBlob.end()) << std::endl;

Sample output which doesn't show what went wrong:

Creating Storage Blob Container Client
Creating container: blobcontainer
Listing blobs...
Blob name: blob1.txt
Blob name: blob2.txt
Blob name: blob3.txt
Downloading Blob

C:\Users\CppDev\source\repos\CppStudySample\out\build\x64-release\CppStudySample.exe (process 11212) exited with code -1073740791.

When the user runs such an app, there is no error message printed.

cc @ronniegeraghty, @Jinming-Hu

ahsonkhan commented 1 year ago

The appropriate exception, reason phrase, and call stack shows up when debugging the application with a breakpoint on blobClient.GetProperties(), but the exception doesn't seem to be thrown when running in release and the information doesn't end up on the console: image

Catching the exception and printing out .what() works as expected, as does looking at the logs:

try {
    auto properties = blobClient.GetProperties().Value;
    std::vector<uint8_t> downloadedBlob(properties.BlobSize);

    blobClient.DownloadTo(downloadedBlob.data(), downloadedBlob.size());
    std::cout << "Downloaded blob contents: " << std::string(downloadedBlob.begin(), downloadedBlob.end()) << std::endl;
}
catch (exception const &ex)
{
    std::cout << ex.what() << std::endl;
}
404 The specified blob does not exist.

Request ID: 1b52a7f6-301e-0066-67a7-95a9f0000000

Environment Logging (AZURE_LOG_LEVEL) turned on to 'Verbose' (in Powershell as $Env:AZURE_LOG_LEVEL='Verbose')

...
[2023-06-02T23:05:43.6778620Z] INFO  : HTTP Response (10ms) : 404 The specified blob does not exist.
date : Fri, 02 Jun 2023 23:05:43 GMT
server : Windows-Azure-Blob/1.0 Microsoft-HTTPAPI/2.0
transfer-encoding : chunked
x-ms-client-request-id : 777e597a-ea0d-4313-8319-91fb19c529d1
x-ms-error-code : REDACTED
x-ms-request-id : b65c2733-201e-0037-31a6-95347c000000
x-ms-version : REDACTED
[2023-06-02T23:05:43.6785021Z] INFO  : HTTP status code 404 won't be retried.
Jinming-Hu commented 1 year ago

If the customer had wrapped the code with a try {...} catch block, he should've see the error message.

I think this is more of a question of capturing exception and printing error message from there. The customer cannot figure out the problem if he doesn't catch exception, no matter which service he's using or the request fails due to whatever reason

LarryOsterman commented 1 year ago

If the customer had wrapped the code with a try {...} catch block, he should've see the error message.

I think this is more of a question of capturing exception and printing error message from there. The customer cannot figure out the problem if he doesn't catch exception, no matter which service he's using or the request fails due to whatever reason

I've seen a similar set of issues with the AMQP samples.

IMHO, because samples are supposed to express optimal customer behavior, they should have a try/catch wrapped around their main which outputs the .what() to cerr. This is especially important on Windows where the unhandled exception behavior is ... suboptimal.

ahsonkhan commented 1 year ago

I agree that we'd want to highlight best practices in our samples, which include try/catch and error handling, since customers certainly start off with copy-paste/modify, and it becomes a great teaching tool (cc @ronniegeraghty as part of docs/samples improvements).

I haven't tried yet, @LarryOsterman have you observed the unhandled exception behavior on linux be better, where we see the contents of the exception, unlike Windows?

LarryOsterman commented 1 year ago

I agree that we'd want to highlight best practices in our samples, which include try/catch and error handling, since customers certainly start off with copy-paste/modify, and it becomes a great teaching tool (cc @ronniegeraghty as part of docs/samples improvements).

I haven't tried yet, @LarryOsterman have you observed the unhandled exception behavior on linux be better, where we see the contents of the exception, unlike Windows?

If my memory serves me correctly, on Linux, we get a diagnostic like "unhandled exception thrown: " followed by ex.what(). On Windows we get a MessageBox with a comment about the application terminating unexpectedly.