feature,fix (device-client) IoTHub Exception for Get and Patch Twin failures #1815

bikamani · 2021-03-05T22:04:30Z

This PR includes changes in follow up to handing Twin failures feature.

Old behavior:
On a failure, a wrong exception (InvalidOperationException) was thrown no matter what the error was.

New behavior:
On a failure, a correct IoTHub exception with status code and message will be thrown in context of Get Twin or Update Reported Properties call.

bikamani · 2021-03-05T22:05:01Z

e2e/test/iothub/twin/TwinE2ETests.cs

-        // These tests worked earlier for Amqp and AmqpWs since it was catching a wrong exception
-        // To Do: Fix Update reported properties method behavior (breaking change) to wait for response
-        // and we should be able to enable these tests then.
-        [Ignore]
        [LoggedTestMethod]


Now these tests can be enabled back in

bikamani · 2021-03-05T22:08:38Z

iothub/device/src/Transport/Amqp/AmqpTransportHandler.cs

+                        }
+                        else
+                        {
+                            task.SetException(ex);


set an exception for task here if there is an error, will be used later.

bikamani · 2021-03-05T22:10:09Z

iothub/device/src/Transport/AmqpIoT/AmqpIoTReceivingLink.cs

                        // Retry for Http status code request timeout, Too many requests and server errors
-                        throw new IotHubException(error, status >= 500 || status == 429 || status == 408);
+                        var exception = new IotHubException(error, status >= 500 || status == 429 || status == 408);
+                        _onTwinMessageReceived.Invoke(null, correlationId, null, exception);


Generated exception is passed through for the action invoked.

bikamani · 2021-03-05T22:10:40Z

iothub/device/src/Transport/Amqp/AmqpConnectionHolder.cs

@@ -36,7 +36,7 @@ public AmqpConnectionHolder(DeviceIdentity deviceIdentity)
        public AmqpUnit CreateAmqpUnit(
            DeviceIdentity deviceIdentity,
            Func<MethodRequestInternal, Task> onMethodCallback,
-            Action<Twin, string, TwinCollection> twinMessageListener,
+            Action<Twin, string, TwinCollection, IotHubException> twinMessageListener,


Adding a new parameter type for Exception

iothub/device/src/Transport/Amqp/AmqpTransportHandler.cs

bikamani · 2021-03-05T23:09:41Z

/azp run

azure-pipelines · 2021-03-05T23:09:51Z

Azure Pipelines successfully started running 1 pipeline(s).

bikamani · 2021-03-05T23:46:18Z

This PR includes changes in follow up to handing Twin failures feature.

Old behavior:
On a failure, a wrong exception (InvalidOperationException) was thrown no matter what the error was.

New behavior:
On a failure, a correct IoTHub exception with status code and message will be thrown in context of Get Twin or Update Reported Properties call.

Related github issue: #1505

drwill-ms · 2021-03-06T19:23:00Z

iothub/device/src/Transport/Amqp/AmqpTransportHandler.cs

@@ -533,7 +539,14 @@ private void TwinMessageListener(Twin twin, string correlationId, TwinCollection
                    TaskCompletionSource<Twin> task;
                    if (_twinResponseCompletions.TryRemove(correlationId, out task))
                    {
-                        task.SetResult(twin);
+                        if(ex == default)


missed a space after if

I'll take care of this in a follow up PR.

* fix: samples readme links were out-of-date (#1784) * fix: samples arguments and remove 1 more dead link (#1785) * fix(readme): Update the location of ConsoleEventListener in our readme * doc(service-client) - Updating readme (#1799) * fix(doc): Update amqp transport exception doc to have detailed description for quota exceeded error mapping * feature(device-client): Make the DeviceClient and ModuleClient extensible (#1802) * doc(service-client): Added extra comments to clarify true and false in dispose (#1805) * feature,fix (device-client) Handle Twin failures using Amqp (#1796) * fix(iot-service): Update xml comments for ServiceClient * fix(iot-device): Update MqttTransportHandler to not use SemaphoreSlim.WaitAsync(TimeSpan, CancellationToken) * fix(iot-device): Update dotnetty task calls to use ConfigureAwait(true) * fix(iot-device): Fix MqttTransportHandler to not await on user supplied C2D callback * IoTHub Exception for Get and Patch Twin failures (#1815) * fix(edge): UnixDomainSocketEndPoint is available in .NET 2.1 and greater (#1816) * UnixDomainSocketEndPoint has been standard since 2.1. Allowing later versions to use the correct class The edge client HSM provider uses UnixDomainSockets (UDS) for communication. Before .NET 2.1 to implement a Unix Socket you had to create your own class to do so. Since 2.1 there has been a native UnixDomainSocketEndPoint class in the runtime. In 2.1 and 3.1 there is no issue. However in 5.0 there are some changes to the way the Socket class handles the native UnixDomainSocketEndPoint class. I didn't dig down extremely deep, but I suspect it's due to the way the endpoint handles the SocketAddress and the string manipulation there seeing as how there is a specific implementation for Windows and for Unix. * feat(e2e) - Enabling soft delete when creating keyvaults (#1820) * fix(e2e) - Change event logging to opt in to specific events only. (#1824) * (service-client): Design for IoT hub AAD authentication * (service-client: Refactor and add implementation for token credential input) (#1781) * (service-client): Refactor and add sas credential (#1786) * (service-client): Add constructors in service client to accept aad and sas tokens. (#1787) * (service-client): Add constructors in registry manager to accept aad and sas tokens. (#1788) * (service-client): Add constructors in job client to accept aad and sas tokens. (#1789) * (service-client): Add constructors to accept aad and sas tokens for digital twins client. (#1790) * fix(service-client): Support for AzureSasCredential for a better user experience (#1797) * doc(service-client): Update readme about the differnt client and operations (#1798) * tests(service-client): E2E tests for aad auth on all our clients (#1800) * test(service-client): Adding e2e tests for sas credential auth for IoT hub. (#1806) * feature(service-client): Adding chaching for aad tokens. (#1807) * fix(service-client)- Add IoT hub token scope. (#1812) Co-authored-by: David R. Williamson <drwill@microsoft.com> Co-authored-by: Abhipsa Misra <abhipsa.misra@microsoft.com> Co-authored-by: bikamani <41314966+bikamani@users.noreply.github.com> Co-authored-by: jamdavi <73593426+jamdavi@users.noreply.github.com>

This reverts commit d96ef56.

* fix: samples readme links were out-of-date (#1784) * fix: samples arguments and remove 1 more dead link (#1785) * fix(readme): Update the location of ConsoleEventListener in our readme * doc(service-client) - Updating readme (#1799) * fix(doc): Update amqp transport exception doc to have detailed description for quota exceeded error mapping * feature(device-client): Make the DeviceClient and ModuleClient extensible (#1802) * doc(service-client): Added extra comments to clarify true and false in dispose (#1805) * feature,fix (device-client) Handle Twin failures using Amqp (#1796) * fix(iot-service): Update xml comments for ServiceClient * fix(iot-device): Update MqttTransportHandler to not use SemaphoreSlim.WaitAsync(TimeSpan, CancellationToken) * fix(iot-device): Update dotnetty task calls to use ConfigureAwait(true) * fix(iot-device): Fix MqttTransportHandler to not await on user supplied C2D callback * IoTHub Exception for Get and Patch Twin failures (#1815) * fix(edge): UnixDomainSocketEndPoint is available in .NET 2.1 and greater (#1816) * UnixDomainSocketEndPoint has been standard since 2.1. Allowing later versions to use the correct class The edge client HSM provider uses UnixDomainSockets (UDS) for communication. Before .NET 2.1 to implement a Unix Socket you had to create your own class to do so. Since 2.1 there has been a native UnixDomainSocketEndPoint class in the runtime. In 2.1 and 3.1 there is no issue. However in 5.0 there are some changes to the way the Socket class handles the native UnixDomainSocketEndPoint class. I didn't dig down extremely deep, but I suspect it's due to the way the endpoint handles the SocketAddress and the string manipulation there seeing as how there is a specific implementation for Windows and for Unix. * feat(e2e) - Enabling soft delete when creating keyvaults (#1820) * fix(e2e) - Change event logging to opt in to specific events only. (#1824) * (service-client): Design for IoT hub AAD authentication * (service-client: Refactor and add implementation for token credential input) (#1781) * (service-client): Refactor and add sas credential (#1786) * (service-client): Add constructors in service client to accept aad and sas tokens. (#1787) * (service-client): Add constructors in registry manager to accept aad and sas tokens. (#1788) * (service-client): Add constructors in job client to accept aad and sas tokens. (#1789) * (service-client): Add constructors to accept aad and sas tokens for digital twins client. (#1790) * fix(service-client): Support for AzureSasCredential for a better user experience (#1797) * doc(service-client): Update readme about the differnt client and operations (#1798) * tests(service-client): E2E tests for aad auth on all our clients (#1800) * test(service-client): Adding e2e tests for sas credential auth for IoT hub. (#1806) * feature(service-client): Adding chaching for aad tokens. (#1807) * fix(service-client)- Add IoT hub token scope. (#1812) Co-authored-by: David R. Williamson <drwill@microsoft.com> Co-authored-by: Abhipsa Misra <abhipsa.misra@microsoft.com> Co-authored-by: bikamani <41314966+bikamani@users.noreply.github.com> Co-authored-by: jamdavi <73593426+jamdavi@users.noreply.github.com>

bikamani added 2 commits March 5, 2021 12:25

Exception handling for Get and Patch

91238e9

fix indentation

4e32b38

bikamani commented Mar 5, 2021

View reviewed changes

bikamani marked this pull request as ready for review March 5, 2021 22:12

bikamani requested review from abhipsaMisra, azabbasi, barustum, drwill-ms, jamdavi, timtay-microsoft and vinagesh as code owners March 5, 2021 22:12

abhipsaMisra reviewed Mar 5, 2021

View reviewed changes

iothub/device/src/Transport/Amqp/AmqpTransportHandler.cs Show resolved Hide resolved

abhipsaMisra approved these changes Mar 5, 2021

View reviewed changes

bikamani merged commit 747f2ac into master Mar 5, 2021

bikamani deleted the bikamani/updateRepImpl branch March 5, 2021 23:38

bikamani changed the title ~~IoTHub Exception for Get and Patch Twin failures~~ feature,fix (device-client) IoTHub Exception for Get and Patch Twin failures Mar 6, 2021

drwill-ms reviewed Mar 6, 2021

View reviewed changes

abhipsaMisra pushed a commit that referenced this pull request Mar 19, 2021

IoTHub Exception for Get and Patch Twin failures (#1815)

d96ef56

abhipsaMisra added a commit that referenced this pull request Mar 19, 2021

Revert "IoTHub Exception for Get and Patch Twin failures (#1815)"

1956607

This reverts commit d96ef56.

abhipsaMisra added a commit that referenced this pull request Mar 19, 2021

Revert "IoTHub Exception for Get and Patch Twin failures (#1815)"

8ac763b

This reverts commit d96ef56.

jamdavi mentioned this pull request Mar 19, 2021

release: Bump versions for latest release #1842

Merged

abhipsaMisra added a commit that referenced this pull request Mar 19, 2021

Revert "IoTHub Exception for Get and Patch Twin failures (#1815)"

51ed20b

This reverts commit d96ef56.

vinagesh pushed a commit that referenced this pull request Mar 22, 2021

IoTHub Exception for Get and Patch Twin failures (#1815)

03afec2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature,fix (device-client) IoTHub Exception for Get and Patch Twin failures #1815

feature,fix (device-client) IoTHub Exception for Get and Patch Twin failures #1815

bikamani commented Mar 5, 2021

bikamani Mar 5, 2021

bikamani Mar 5, 2021

bikamani Mar 5, 2021

bikamani Mar 5, 2021

bikamani commented Mar 5, 2021

azure-pipelines bot commented Mar 5, 2021

bikamani commented Mar 5, 2021

drwill-ms Mar 6, 2021

bikamani Mar 8, 2021

feature,fix (device-client) IoTHub Exception for Get and Patch Twin failures #1815

feature,fix (device-client) IoTHub Exception for Get and Patch Twin failures #1815

Conversation

bikamani commented Mar 5, 2021

bikamani Mar 5, 2021

Choose a reason for hiding this comment

bikamani Mar 5, 2021

Choose a reason for hiding this comment

bikamani Mar 5, 2021

Choose a reason for hiding this comment

bikamani Mar 5, 2021

Choose a reason for hiding this comment

bikamani commented Mar 5, 2021

azure-pipelines bot commented Mar 5, 2021

bikamani commented Mar 5, 2021

drwill-ms Mar 6, 2021

Choose a reason for hiding this comment

bikamani Mar 8, 2021

Choose a reason for hiding this comment