search

Azure / azure-iot-sdk-c

A C99 SDK for connecting devices to Microsoft Azure IoT services
https://azure.github.io/azure-iot-sdk-c
Other
588 stars 737 forks source link

SAS request error for blob upload cannot be handled #2614

Closed opekhovskiy-zf closed 6 months ago

opekhovskiy-zf commented 7 months ago

Development Machine, OS, Compiler (and Other Relevant Toolchain Info)

Ubuntu 22.04.1 LTS gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0

SDK Version

1.11.0 Commit SHA: 97fef570416467598100b782ef27ceadad9ca796 Release 2023-08-07

Protocol

MQTT

Describe the Bug

There is no way to get error status on SAS request for blob upload.

So when we call IoTHubDeviceClient_UploadMultipleBlocksToBlobAsync, it invokes the following chain: IoTHubDeviceClient_UploadMultipleBlocksToBlobAsync IoTHubClientCore_UploadMultipleBlocksToBlobAsync startHttpWorkerThread ThreadAPI_Create [here we lose error returned by uploadMultipleBlock_thread] uploadMultipleBlock_thread [int, IOTHUB_CLIENT_ERROR] IoTHubClientCore_LL_UploadMultipleBlocksToBlobEx [IOTHUB_CLIENT_RESULT, IOTHUB_CLIENT_ERROR] IoTHubClient_LL_UploadToBlob_InitializeUpload [IOTHUB_CLIENT_RESULT, IOTHUB_CLIENT_ERROR] IoTHubClient_LL_UploadToBlob_GetBlobCredentialsFromIoTHub [int, MU_FAILURE] send_http_sas_request [int, MU_FAILURE] HTTPAPIEX_SAS_ExecuteRequest

SAS request is asynchronous, so we cannot get result immediately. And we never call IOTHUB_CLIENT_FILE_UPLOAD_GET_DATA_CALLBACK_EX with this error. So right now the calling routine waits forever.

Moreover even synchronous call doesn't return an error because invocation of ThreadAPI_Join (that returns exit code of a thread function) just passes unused variable for return code, e.g. in the following chain. IoTHubDeviceClient_Destroy IoTHubClientCore_Destroy

Seems like the best way to inform the caller routine about the error is to invocate IOTHUB_CLIENT_FILE_UPLOAD_GET_DATA_CALLBACK_EX (and IOTHUB_CLIENT_FILE_UPLOAD_GET_DATA_CALLBACK) function and pass the error through IOTHUB_CLIENT_FILE_UPLOAD_RESULT result parameter.

ewertons commented 6 months ago

Hi @opekhovskiy-zf you are correct, we apologize for that error. This issue is somehow related to https://github.com/Azure/azure-iot-sdk-c/issues/2569, and a fix for both is being worked on.

ewertons commented 6 months ago

Indeed the branch with the proposed fix is here: https://github.com/Azure/azure-iot-sdk-c/tree/ewertons/FixThreadedUploadToBlob

In summary, we are trapping both the return code from IoTHubClientCore_LL_UploadMultipleBlocksToBlobEx and callback result (if invoked) and firing the IOTHUB_CLIENT_FILE_UPLOAD_GET_DATA_CALLBACK_EX from uploadMultipleBlock_thread after calling IoTHubClientCore_LL_UploadMultipleBlocksToBlobEx with the composite result. Would you be able to validate this branch as well before we move to merge it?

ewertons commented 6 months ago

A change has been merged to address this issue. Please feel free to reopen this case if you would like to follow up. Thanks, Azure IoT SDKs Team.