From b13833dd9bad65dc8d829e793bd373697ee09238 Mon Sep 17 00:00:00 2001 From: timtay-microsoft Date: Thu, 6 Apr 2023 14:52:05 -0700 Subject: [PATCH 1/4] Simplify/rename error class given to IoT hub service client error processors Instead of provided either an IOException or an IotHubServiceException, we now just provide a single Exception that can be either of those types or more. Also create some facade classes around ErrorContext so that the error processors of each of the AMQP using clients' error processor is unique --- iothub/service/src/Amqp/AmqpClientHelper.cs | 4 +- .../src/Feedback/FeedbackMessagingError.cs | 18 ++++++++ .../MessageFeedbackProcessorClient.cs | 26 ++++------- .../FileUpload/FileUploadNotificationError.cs | 18 ++++++++ .../FileUploadNotificationProcessorClient.cs | 28 +++++------- .../service/src/Messaging/MessagesClient.cs | 20 ++++----- .../service/src/Messaging/MessagingError.cs | 18 ++++++++ .../src/Messaging/Models/ErrorContext.cs | 45 +++---------------- 8 files changed, 92 insertions(+), 85 deletions(-) create mode 100644 iothub/service/src/Feedback/FeedbackMessagingError.cs create mode 100644 iothub/service/src/FileUpload/FileUploadNotificationError.cs create mode 100644 iothub/service/src/Messaging/MessagingError.cs diff --git a/iothub/service/src/Amqp/AmqpClientHelper.cs b/iothub/service/src/Amqp/AmqpClientHelper.cs index d2734eddf6..31c4860770 100644 --- a/iothub/service/src/Amqp/AmqpClientHelper.cs +++ b/iothub/service/src/Amqp/AmqpClientHelper.cs @@ -147,9 +147,9 @@ internal static IotHubServiceException ToIotHubClientContract(Error error, Excep return retException; } - internal static ErrorContext GetErrorContextFromException(AmqpException exception) + internal static IotHubServiceException GetIotHubExceptionFromAmqpException(AmqpException exception) { - return new ErrorContext(new IotHubServiceException(exception.Error.ToString(), exception)); + return new IotHubServiceException(exception.Error.ToString(), exception); } } } diff --git a/iothub/service/src/Feedback/FeedbackMessagingError.cs b/iothub/service/src/Feedback/FeedbackMessagingError.cs new file mode 100644 index 0000000000..292bcee810 --- /dev/null +++ b/iothub/service/src/Feedback/FeedbackMessagingError.cs @@ -0,0 +1,18 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; + +namespace Microsoft.Azure.Devices +{ + /// + /// The context provided to the error processor for a connection loss event or other failure + /// when using the . + /// + public class FeedbackMessagingError : ErrorContext + { + internal FeedbackMessagingError(Exception exception) : base(exception) + { + } + } +} diff --git a/iothub/service/src/Feedback/MessageFeedbackProcessorClient.cs b/iothub/service/src/Feedback/MessageFeedbackProcessorClient.cs index 55548a2747..b106a836c4 100644 --- a/iothub/service/src/Feedback/MessageFeedbackProcessorClient.cs +++ b/iothub/service/src/Feedback/MessageFeedbackProcessorClient.cs @@ -99,9 +99,9 @@ internal MessageFeedbackProcessorClient( /// /// //... /// - /// public void OnConnectionLost(ErrorContext errorContext) + /// public void OnConnectionLost(FeedbackMessagingError error) /// { - /// Console.WriteLine("Feedback message processor connection lost") + /// Console.WriteLine("Feedback message processor connection lost. Error: {error.Exception.Message}") /// /// // Add reconnection logic as needed, for example: /// await serviceClient.MessageFeedbackProcessor.OpenAsync(); @@ -113,7 +113,7 @@ internal MessageFeedbackProcessorClient( /// This callback will start receiving events again once is called. /// This callback will persist across any number of open/close/open calls, so it does not need to be set before each open call. /// - public Func ErrorProcessor { get; set; } + public Func ErrorProcessor { get; set; } /// /// Open the connection and start receiving acknowledgements for messages sent. @@ -249,14 +249,7 @@ private async void OnFeedbackMessageReceivedAsync(AmqpMessage amqpMessage) try { - if (ex is IotHubServiceException hubEx) - { - ErrorProcessor?.Invoke(new ErrorContext(hubEx)); - } - else if (ex is IOException ioEx) - { - ErrorProcessor?.Invoke(new ErrorContext(ioEx)); - } + ErrorProcessor?.Invoke(new FeedbackMessagingError(ex)); await _amqpConnection.AbandonMessageAsync(amqpMessage.DeliveryTag).ConfigureAwait(false); } @@ -276,18 +269,17 @@ private void OnConnectionClosed(object sender, EventArgs e) { if (((AmqpObject)sender).TerminalException is AmqpException exception) { - ErrorContext errorContext = AmqpClientHelper.GetErrorContextFromException(exception); - ErrorProcessor?.Invoke(errorContext); - Exception exceptionToLog = errorContext.IotHubServiceException; + IotHubServiceException mappedException = AmqpClientHelper.GetIotHubExceptionFromAmqpException(exception); + ErrorProcessor?.Invoke(new FeedbackMessagingError(mappedException)); if (Logging.IsEnabled) - Logging.Error(this, $"{nameof(sender)}.{nameof(OnConnectionClosed)} threw an exception: {exceptionToLog}", nameof(OnConnectionClosed)); + Logging.Error(this, $"{nameof(sender)}.{nameof(OnConnectionClosed)} threw an exception: {mappedException}", nameof(OnConnectionClosed)); } else { var defaultException = new IotHubServiceException("AMQP connection was lost.", ((AmqpObject)sender).TerminalException); - var errorContext = new ErrorContext(defaultException); - ErrorProcessor?.Invoke(errorContext); + var error = new FeedbackMessagingError(defaultException); + ErrorProcessor?.Invoke(error); if (Logging.IsEnabled) Logging.Error(this, $"{nameof(sender)}.{nameof(OnConnectionClosed)} threw an exception: {defaultException}", nameof(OnConnectionClosed)); diff --git a/iothub/service/src/FileUpload/FileUploadNotificationError.cs b/iothub/service/src/FileUpload/FileUploadNotificationError.cs new file mode 100644 index 0000000000..12082003a1 --- /dev/null +++ b/iothub/service/src/FileUpload/FileUploadNotificationError.cs @@ -0,0 +1,18 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; + +namespace Microsoft.Azure.Devices +{ + /// + /// The context provided to the error processor for a connection loss event or other failure + /// when using the . + /// + public class FileUploadNotificationError : ErrorContext + { + internal FileUploadNotificationError(Exception exception) : base(exception) + { + } + } +} diff --git a/iothub/service/src/FileUpload/FileUploadNotificationProcessorClient.cs b/iothub/service/src/FileUpload/FileUploadNotificationProcessorClient.cs index 44815aaed3..2193fa9cce 100644 --- a/iothub/service/src/FileUpload/FileUploadNotificationProcessorClient.cs +++ b/iothub/service/src/FileUpload/FileUploadNotificationProcessorClient.cs @@ -99,9 +99,9 @@ internal FileUploadNotificationProcessorClient( /// /// //... /// - /// public async Task OnConnectionLostAsync(ErrorContext errorContext) + /// public async Task OnConnectionLostAsync(FileUploadNotificationError error) /// { - /// Console.WriteLine("File upload notification processor connection lost"); + /// Console.WriteLine("File upload notification processor connection lost. Error: {error.Exception.Message}"); /// /// // Add reconnection logic as needed, for example: /// await serviceClient.FileUploadNotificationProcessor.OpenAsync(); @@ -113,7 +113,7 @@ internal FileUploadNotificationProcessorClient( /// This callback will start receiving events again once is called. /// This callback will persist across any number of open/close/open calls, so it does not need to be set before each open call. /// - public Func ErrorProcessor { get; set; } + public Func ErrorProcessor { get; set; } /// /// Open the connection and start receiving file upload notifications. @@ -239,14 +239,7 @@ private async void OnNotificationMessageReceivedAsync(AmqpMessage amqpMessage) { try { - if (ex is IotHubServiceException hubEx) - { - await ErrorProcessor.Invoke(new ErrorContext(hubEx)).ConfigureAwait(false); - } - else if (ex is IOException ioEx) - { - await ErrorProcessor.Invoke(new ErrorContext(ioEx)).ConfigureAwait(false); - } + await ErrorProcessor.Invoke(new FileUploadNotificationError(ex)).ConfigureAwait(false); } catch (Exception ex3) { @@ -280,20 +273,19 @@ private void OnConnectionClosed(object sender, EventArgs e) { if (((AmqpObject)sender).TerminalException is AmqpException exception) { - ErrorContext errorContext = AmqpClientHelper.GetErrorContextFromException(exception); - ErrorProcessor?.Invoke(errorContext); + IotHubServiceException mappedException = AmqpClientHelper.GetIotHubExceptionFromAmqpException(exception); + ErrorProcessor?.Invoke(new FileUploadNotificationError(mappedException)); if (Logging.IsEnabled) { - Exception exceptionToLog = errorContext.IotHubServiceException; - Logging.Error(this, $"{nameof(sender)}.{nameof(OnConnectionClosed)} threw an exception: {exceptionToLog}", nameof(OnConnectionClosed)); + Logging.Error(this, $"{nameof(sender)}.{nameof(OnConnectionClosed)} threw an exception: {mappedException}", nameof(OnConnectionClosed)); } } else { - var defaultException = new IotHubServiceException("AMQP connection was lost", ((AmqpObject)sender).TerminalException); - var errorContext = new ErrorContext(defaultException); - ErrorProcessor?.Invoke(errorContext); + var defaultException = new IOException("AMQP connection was lost", ((AmqpObject)sender).TerminalException); + var error = new FileUploadNotificationError(defaultException); + ErrorProcessor?.Invoke(error); if (Logging.IsEnabled) Logging.Error(this, $"{nameof(sender)}.{nameof(OnConnectionClosed)} threw an exception: {defaultException}", nameof(OnConnectionClosed)); diff --git a/iothub/service/src/Messaging/MessagesClient.cs b/iothub/service/src/Messaging/MessagesClient.cs index 42985a69ca..1a277d7b7a 100644 --- a/iothub/service/src/Messaging/MessagesClient.cs +++ b/iothub/service/src/Messaging/MessagesClient.cs @@ -3,6 +3,7 @@ using System; using System.Globalization; +using System.IO; using System.Net; using System.Net.Http; using System.Threading; @@ -85,9 +86,9 @@ internal MessagesClient( /// /// //... /// - /// public void OnConnectionLost(ErrorContext errorContext) + /// public void OnConnectionLost(MessagingError error) /// { - /// Console.WriteLine("Messaging client connection lost"); + /// Console.WriteLine($"Messaging client connection lost. Error: {error.Exception.Message}") /// /// // Add reconnection logic as needed, for example: /// await serviceClient.Messaging.OpenAsync(); @@ -99,7 +100,7 @@ internal MessagesClient( /// This callback will start receiving events again once is called. /// This callback will persist across any number of open/close/open calls, so it does not need to be set before each open call. /// - public Func ErrorProcessor { get; set; } + public Func ErrorProcessor { get; set; } /// /// Open the connection. Must be done before any cloud-to-device messages can be sent. @@ -391,17 +392,16 @@ private void OnConnectionClosed(object sender, EventArgs e) { if (((AmqpObject)sender).TerminalException is AmqpException exception) { - ErrorContext errorContext = AmqpClientHelper.GetErrorContextFromException(exception); - ErrorProcessor?.Invoke(errorContext); - Exception exceptionToLog = errorContext.IotHubServiceException; + IotHubServiceException mappedException = AmqpClientHelper.GetIotHubExceptionFromAmqpException(exception); + ErrorProcessor?.Invoke(new MessagingError(mappedException)); if (Logging.IsEnabled) - Logging.Error(this, $"{nameof(sender)}.{nameof(OnConnectionClosed)} threw an exception: {exceptionToLog}", nameof(OnConnectionClosed)); + Logging.Error(this, $"{nameof(sender)}.{nameof(OnConnectionClosed)} threw an exception: {mappedException}", nameof(OnConnectionClosed)); } else { - var defaultException = new IotHubServiceException("AMQP connection was lost", ((AmqpObject)sender).TerminalException); - var errorContext = new ErrorContext(defaultException); - ErrorProcessor?.Invoke(errorContext); + var defaultException = new IOException("AMQP connection was lost", ((AmqpObject)sender).TerminalException); + var error = new MessagingError(defaultException); + ErrorProcessor?.Invoke(error); if (Logging.IsEnabled) Logging.Error(this, $"{nameof(sender)}.{nameof(OnConnectionClosed)} threw an exception: {defaultException}", nameof(OnConnectionClosed)); } diff --git a/iothub/service/src/Messaging/MessagingError.cs b/iothub/service/src/Messaging/MessagingError.cs new file mode 100644 index 0000000000..f624d59bdb --- /dev/null +++ b/iothub/service/src/Messaging/MessagingError.cs @@ -0,0 +1,18 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; + +namespace Microsoft.Azure.Devices +{ + /// + /// The context provided to the error processor for a connection loss event or other failure + /// when using the . + /// + public class MessagingError : ErrorContext + { + internal MessagingError(Exception exception) : base(exception) + { + } + } +} diff --git a/iothub/service/src/Messaging/Models/ErrorContext.cs b/iothub/service/src/Messaging/Models/ErrorContext.cs index e97486ce35..e00e3b7ff9 100644 --- a/iothub/service/src/Messaging/Models/ErrorContext.cs +++ b/iothub/service/src/Messaging/Models/ErrorContext.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System; using System.IO; namespace Microsoft.Azure.Devices @@ -9,52 +10,20 @@ namespace Microsoft.Azure.Devices /// The context for a given connection loss event for , /// , and . /// - /// - /// The context includes the cause of the connection loss and makes a distinction between network - /// level issues(no internet) and IoT Hub level issues(resource not found, throttling, internal server error, etc.). - /// public class ErrorContext { - /// - /// The IoT hub level exception, if any IoT hub level exception caused this connection loss. - /// - /// - /// For example, if the device does not exist. - /// - /// - internal ErrorContext(IotHubServiceException iotHubServiceException) - { - IotHubServiceException = iotHubServiceException; - } - - /// - /// The network level exception, if any network level exception caused this connection loss. - /// - /// - /// For example, if the device has no internet connection. - /// - /// - internal ErrorContext(IOException ioException) + internal ErrorContext(Exception exception) { - IOException = ioException; + Exception = exception; } /// - /// The IoT hub level exception, if any IoT hub level exception caused this connection loss. - /// - /// - /// For example, if you attempt to send a cloud-to-device message to a device that does not exist. if this exception is null, - /// then will not be null. - /// - public IotHubServiceException IotHubServiceException { get; } - - /// - /// The network level exception, if any network level exception caused this connection loss. + /// The exception that caused the error processor execute. /// /// - /// For example, if you attempt to send a cloud-to-device message to a device when your device has no internet connection. - /// If this exception is null, then will not be null. + /// This exception is usually either of type or + /// . /// - public IOException IOException { get; } + public Exception Exception { get; } } } From cebe413a704c25ffa8a28fb3922774adf382c307 Mon Sep 17 00:00:00 2001 From: timtay-microsoft Date: Thu, 6 Apr 2023 14:57:29 -0700 Subject: [PATCH 2/4] fixup --- e2e/Tests/iothub/service/FileUploadNotificationE2ETest.cs | 2 +- .../iothub/service/MessageFeedbackReceiverE2ETest.cs | 2 +- e2e/Tests/iothub/service/MessagingClientE2ETests.cs | 2 +- .../src/Feedback/MessageFeedbackProcessorClient.cs | 8 ++++---- ...MessagingError.cs => MessageFeedbackProcessorError.cs} | 4 ++-- .../FileUpload/FileUploadNotificationProcessorClient.cs | 8 ++++---- ...onError.cs => FileUploadNotificationProcessorError.cs} | 4 ++-- iothub/service/src/Messaging/MessagesClient.cs | 6 +++--- .../{MessagingError.cs => MessagesClientError.cs} | 4 ++-- iothub/service/tests/Amqp/AmqpClientHelperTests.cs | 7 ++++--- 10 files changed, 24 insertions(+), 23 deletions(-) rename iothub/service/src/Feedback/{FeedbackMessagingError.cs => MessageFeedbackProcessorError.cs} (75%) rename iothub/service/src/FileUpload/{FileUploadNotificationError.cs => FileUploadNotificationProcessorError.cs} (73%) rename iothub/service/src/Messaging/{MessagingError.cs => MessagesClientError.cs} (76%) diff --git a/e2e/Tests/iothub/service/FileUploadNotificationE2ETest.cs b/e2e/Tests/iothub/service/FileUploadNotificationE2ETest.cs index 0a4cda7e0d..6003c45224 100644 --- a/e2e/Tests/iothub/service/FileUploadNotificationE2ETest.cs +++ b/e2e/Tests/iothub/service/FileUploadNotificationE2ETest.cs @@ -124,7 +124,7 @@ public async Task FileUploadNotification_CloseGracefully_DoesNotExecuteConnectio // arrange using var sender = new IotHubServiceClient(TestConfiguration.IotHub.ConnectionString); bool connectionLossEventExecuted = false; - Func OnConnectionLost = delegate + Func OnConnectionLost = delegate { // There is a small chance that this test's connection is interrupted by an actual // network failure (when this callback should be executed), but the operations diff --git a/e2e/Tests/iothub/service/MessageFeedbackReceiverE2ETest.cs b/e2e/Tests/iothub/service/MessageFeedbackReceiverE2ETest.cs index e427547870..2ed17b3bc2 100644 --- a/e2e/Tests/iothub/service/MessageFeedbackReceiverE2ETest.cs +++ b/e2e/Tests/iothub/service/MessageFeedbackReceiverE2ETest.cs @@ -107,7 +107,7 @@ public async Task MessageFeedbackReceiver_CloseGracefully_DoesNotExecuteConnecti // arrange using var sender = new IotHubServiceClient(TestConfiguration.IotHub.ConnectionString); bool connectionLossEventExecuted = false; - Func OnConnectionLost = delegate + Func OnConnectionLost = delegate { // There is a small chance that this test's connection is interrupted by an actual // network failure (when this callback should be executed), but the operations diff --git a/e2e/Tests/iothub/service/MessagingClientE2ETests.cs b/e2e/Tests/iothub/service/MessagingClientE2ETests.cs index af15aefa09..4ca5b3c3a6 100644 --- a/e2e/Tests/iothub/service/MessagingClientE2ETests.cs +++ b/e2e/Tests/iothub/service/MessagingClientE2ETests.cs @@ -424,7 +424,7 @@ public async Task MessagingClient_CloseGracefully_DoesNotExecuteConnectionLoss() // arrange using var sender = new IotHubServiceClient(TestConfiguration.IotHub.ConnectionString); bool connectionLossEventExecuted = false; - Func OnConnectionLost = delegate + Func OnConnectionLost = delegate { // There is a small chance that this test's connection is interrupted by an actual // network failure (when this callback should be executed), but the operations diff --git a/iothub/service/src/Feedback/MessageFeedbackProcessorClient.cs b/iothub/service/src/Feedback/MessageFeedbackProcessorClient.cs index b106a836c4..4131aff557 100644 --- a/iothub/service/src/Feedback/MessageFeedbackProcessorClient.cs +++ b/iothub/service/src/Feedback/MessageFeedbackProcessorClient.cs @@ -113,7 +113,7 @@ internal MessageFeedbackProcessorClient( /// This callback will start receiving events again once is called. /// This callback will persist across any number of open/close/open calls, so it does not need to be set before each open call. /// - public Func ErrorProcessor { get; set; } + public Func ErrorProcessor { get; set; } /// /// Open the connection and start receiving acknowledgements for messages sent. @@ -249,7 +249,7 @@ private async void OnFeedbackMessageReceivedAsync(AmqpMessage amqpMessage) try { - ErrorProcessor?.Invoke(new FeedbackMessagingError(ex)); + ErrorProcessor?.Invoke(new MessageFeedbackProcessorError(ex)); await _amqpConnection.AbandonMessageAsync(amqpMessage.DeliveryTag).ConfigureAwait(false); } @@ -270,7 +270,7 @@ private void OnConnectionClosed(object sender, EventArgs e) if (((AmqpObject)sender).TerminalException is AmqpException exception) { IotHubServiceException mappedException = AmqpClientHelper.GetIotHubExceptionFromAmqpException(exception); - ErrorProcessor?.Invoke(new FeedbackMessagingError(mappedException)); + ErrorProcessor?.Invoke(new MessageFeedbackProcessorError(mappedException)); if (Logging.IsEnabled) Logging.Error(this, $"{nameof(sender)}.{nameof(OnConnectionClosed)} threw an exception: {mappedException}", nameof(OnConnectionClosed)); @@ -278,7 +278,7 @@ private void OnConnectionClosed(object sender, EventArgs e) else { var defaultException = new IotHubServiceException("AMQP connection was lost.", ((AmqpObject)sender).TerminalException); - var error = new FeedbackMessagingError(defaultException); + var error = new MessageFeedbackProcessorError(defaultException); ErrorProcessor?.Invoke(error); if (Logging.IsEnabled) diff --git a/iothub/service/src/Feedback/FeedbackMessagingError.cs b/iothub/service/src/Feedback/MessageFeedbackProcessorError.cs similarity index 75% rename from iothub/service/src/Feedback/FeedbackMessagingError.cs rename to iothub/service/src/Feedback/MessageFeedbackProcessorError.cs index 292bcee810..34c4625f8e 100644 --- a/iothub/service/src/Feedback/FeedbackMessagingError.cs +++ b/iothub/service/src/Feedback/MessageFeedbackProcessorError.cs @@ -9,9 +9,9 @@ namespace Microsoft.Azure.Devices /// The context provided to the error processor for a connection loss event or other failure /// when using the . /// - public class FeedbackMessagingError : ErrorContext + public class MessageFeedbackProcessorError : ErrorContext { - internal FeedbackMessagingError(Exception exception) : base(exception) + internal MessageFeedbackProcessorError(Exception exception) : base(exception) { } } diff --git a/iothub/service/src/FileUpload/FileUploadNotificationProcessorClient.cs b/iothub/service/src/FileUpload/FileUploadNotificationProcessorClient.cs index 2193fa9cce..47dbb68fef 100644 --- a/iothub/service/src/FileUpload/FileUploadNotificationProcessorClient.cs +++ b/iothub/service/src/FileUpload/FileUploadNotificationProcessorClient.cs @@ -113,7 +113,7 @@ internal FileUploadNotificationProcessorClient( /// This callback will start receiving events again once is called. /// This callback will persist across any number of open/close/open calls, so it does not need to be set before each open call. /// - public Func ErrorProcessor { get; set; } + public Func ErrorProcessor { get; set; } /// /// Open the connection and start receiving file upload notifications. @@ -239,7 +239,7 @@ private async void OnNotificationMessageReceivedAsync(AmqpMessage amqpMessage) { try { - await ErrorProcessor.Invoke(new FileUploadNotificationError(ex)).ConfigureAwait(false); + await ErrorProcessor.Invoke(new FileUploadNotificationProcessorError(ex)).ConfigureAwait(false); } catch (Exception ex3) { @@ -274,7 +274,7 @@ private void OnConnectionClosed(object sender, EventArgs e) if (((AmqpObject)sender).TerminalException is AmqpException exception) { IotHubServiceException mappedException = AmqpClientHelper.GetIotHubExceptionFromAmqpException(exception); - ErrorProcessor?.Invoke(new FileUploadNotificationError(mappedException)); + ErrorProcessor?.Invoke(new FileUploadNotificationProcessorError(mappedException)); if (Logging.IsEnabled) { @@ -284,7 +284,7 @@ private void OnConnectionClosed(object sender, EventArgs e) else { var defaultException = new IOException("AMQP connection was lost", ((AmqpObject)sender).TerminalException); - var error = new FileUploadNotificationError(defaultException); + var error = new FileUploadNotificationProcessorError(defaultException); ErrorProcessor?.Invoke(error); if (Logging.IsEnabled) diff --git a/iothub/service/src/FileUpload/FileUploadNotificationError.cs b/iothub/service/src/FileUpload/FileUploadNotificationProcessorError.cs similarity index 73% rename from iothub/service/src/FileUpload/FileUploadNotificationError.cs rename to iothub/service/src/FileUpload/FileUploadNotificationProcessorError.cs index 12082003a1..c527ed66f0 100644 --- a/iothub/service/src/FileUpload/FileUploadNotificationError.cs +++ b/iothub/service/src/FileUpload/FileUploadNotificationProcessorError.cs @@ -9,9 +9,9 @@ namespace Microsoft.Azure.Devices /// The context provided to the error processor for a connection loss event or other failure /// when using the . /// - public class FileUploadNotificationError : ErrorContext + public class FileUploadNotificationProcessorError : ErrorContext { - internal FileUploadNotificationError(Exception exception) : base(exception) + internal FileUploadNotificationProcessorError(Exception exception) : base(exception) { } } diff --git a/iothub/service/src/Messaging/MessagesClient.cs b/iothub/service/src/Messaging/MessagesClient.cs index 1a277d7b7a..a98c42556a 100644 --- a/iothub/service/src/Messaging/MessagesClient.cs +++ b/iothub/service/src/Messaging/MessagesClient.cs @@ -100,7 +100,7 @@ internal MessagesClient( /// This callback will start receiving events again once is called. /// This callback will persist across any number of open/close/open calls, so it does not need to be set before each open call. /// - public Func ErrorProcessor { get; set; } + public Func ErrorProcessor { get; set; } /// /// Open the connection. Must be done before any cloud-to-device messages can be sent. @@ -393,14 +393,14 @@ private void OnConnectionClosed(object sender, EventArgs e) if (((AmqpObject)sender).TerminalException is AmqpException exception) { IotHubServiceException mappedException = AmqpClientHelper.GetIotHubExceptionFromAmqpException(exception); - ErrorProcessor?.Invoke(new MessagingError(mappedException)); + ErrorProcessor?.Invoke(new MessagesClientError(mappedException)); if (Logging.IsEnabled) Logging.Error(this, $"{nameof(sender)}.{nameof(OnConnectionClosed)} threw an exception: {mappedException}", nameof(OnConnectionClosed)); } else { var defaultException = new IOException("AMQP connection was lost", ((AmqpObject)sender).TerminalException); - var error = new MessagingError(defaultException); + var error = new MessagesClientError(defaultException); ErrorProcessor?.Invoke(error); if (Logging.IsEnabled) Logging.Error(this, $"{nameof(sender)}.{nameof(OnConnectionClosed)} threw an exception: {defaultException}", nameof(OnConnectionClosed)); diff --git a/iothub/service/src/Messaging/MessagingError.cs b/iothub/service/src/Messaging/MessagesClientError.cs similarity index 76% rename from iothub/service/src/Messaging/MessagingError.cs rename to iothub/service/src/Messaging/MessagesClientError.cs index f624d59bdb..ac91e3c0c7 100644 --- a/iothub/service/src/Messaging/MessagingError.cs +++ b/iothub/service/src/Messaging/MessagesClientError.cs @@ -9,9 +9,9 @@ namespace Microsoft.Azure.Devices /// The context provided to the error processor for a connection loss event or other failure /// when using the . /// - public class MessagingError : ErrorContext + public class MessagesClientError : ErrorContext { - internal MessagingError(Exception exception) : base(exception) + internal MessagesClientError(Exception exception) : base(exception) { } } diff --git a/iothub/service/tests/Amqp/AmqpClientHelperTests.cs b/iothub/service/tests/Amqp/AmqpClientHelperTests.cs index 9b710deefd..094c8d5183 100644 --- a/iothub/service/tests/Amqp/AmqpClientHelperTests.cs +++ b/iothub/service/tests/Amqp/AmqpClientHelperTests.cs @@ -257,11 +257,12 @@ public void AmqpClientHelper_GetErrorContextFromException_Success() var amqpException = new AmqpException(error); // act - ErrorContext act = AmqpClientHelper.GetErrorContextFromException(amqpException); + Exception act = AmqpClientHelper.GetIotHubExceptionFromAmqpException(amqpException); // assert - act.IotHubServiceException.Message.Should().Be(error.ToString()); - act.IotHubServiceException.InnerException.Should().BeEquivalentTo(amqpException); + act.Should().BeOfType(typeof(IotHubServiceException)); + ((IotHubServiceException)act).Message.Should().Be(error.ToString()); + ((IotHubServiceException)act).InnerException.Should().BeEquivalentTo(amqpException); } } } From fd8b3507356afd73186ba9280d416c1afc6d85a3 Mon Sep 17 00:00:00 2001 From: timtay-microsoft Date: Thu, 6 Apr 2023 16:54:42 -0700 Subject: [PATCH 3/4] feedback --- .../MessageFeedbackProcessorClient.cs | 2 +- .../Feedback/MessageFeedbackProcessorError.cs | 15 ++++++++-- .../FileUploadNotificationProcessorError.cs | 15 ++++++++-- .../service/src/Messaging/MessagesClient.cs | 2 +- .../src/Messaging/MessagesClientError.cs | 15 ++++++++-- .../src/Messaging/Models/ErrorContext.cs | 29 ------------------- 6 files changed, 41 insertions(+), 37 deletions(-) delete mode 100644 iothub/service/src/Messaging/Models/ErrorContext.cs diff --git a/iothub/service/src/Feedback/MessageFeedbackProcessorClient.cs b/iothub/service/src/Feedback/MessageFeedbackProcessorClient.cs index 4131aff557..ce6cc3ab15 100644 --- a/iothub/service/src/Feedback/MessageFeedbackProcessorClient.cs +++ b/iothub/service/src/Feedback/MessageFeedbackProcessorClient.cs @@ -99,7 +99,7 @@ internal MessageFeedbackProcessorClient( /// /// //... /// - /// public void OnConnectionLost(FeedbackMessagingError error) + /// public async Task OnConnectionLost(FeedbackMessagingError error) /// { /// Console.WriteLine("Feedback message processor connection lost. Error: {error.Exception.Message}") /// diff --git a/iothub/service/src/Feedback/MessageFeedbackProcessorError.cs b/iothub/service/src/Feedback/MessageFeedbackProcessorError.cs index 34c4625f8e..718e208ba7 100644 --- a/iothub/service/src/Feedback/MessageFeedbackProcessorError.cs +++ b/iothub/service/src/Feedback/MessageFeedbackProcessorError.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.IO; namespace Microsoft.Azure.Devices { @@ -9,10 +10,20 @@ namespace Microsoft.Azure.Devices /// The context provided to the error processor for a connection loss event or other failure /// when using the . /// - public class MessageFeedbackProcessorError : ErrorContext + public class MessageFeedbackProcessorError { - internal MessageFeedbackProcessorError(Exception exception) : base(exception) + internal MessageFeedbackProcessorError(Exception exception) { + Exception = exception; } + + /// + /// The exception that caused the error processor execute. + /// + /// + /// This exception is usually either of type or + /// . + /// + public Exception Exception { get; } } } diff --git a/iothub/service/src/FileUpload/FileUploadNotificationProcessorError.cs b/iothub/service/src/FileUpload/FileUploadNotificationProcessorError.cs index c527ed66f0..9cd77d2fcc 100644 --- a/iothub/service/src/FileUpload/FileUploadNotificationProcessorError.cs +++ b/iothub/service/src/FileUpload/FileUploadNotificationProcessorError.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.IO; namespace Microsoft.Azure.Devices { @@ -9,10 +10,20 @@ namespace Microsoft.Azure.Devices /// The context provided to the error processor for a connection loss event or other failure /// when using the . /// - public class FileUploadNotificationProcessorError : ErrorContext + public class FileUploadNotificationProcessorError { - internal FileUploadNotificationProcessorError(Exception exception) : base(exception) + internal FileUploadNotificationProcessorError(Exception exception) { + Exception = exception; } + + /// + /// The exception that caused the error processor execute. + /// + /// + /// This exception is usually either of type or + /// . + /// + public Exception Exception { get; } } } diff --git a/iothub/service/src/Messaging/MessagesClient.cs b/iothub/service/src/Messaging/MessagesClient.cs index a98c42556a..b4a616555e 100644 --- a/iothub/service/src/Messaging/MessagesClient.cs +++ b/iothub/service/src/Messaging/MessagesClient.cs @@ -86,7 +86,7 @@ internal MessagesClient( /// /// //... /// - /// public void OnConnectionLost(MessagingError error) + /// public async Task OnConnectionLost(MessagingError error) /// { /// Console.WriteLine($"Messaging client connection lost. Error: {error.Exception.Message}") /// diff --git a/iothub/service/src/Messaging/MessagesClientError.cs b/iothub/service/src/Messaging/MessagesClientError.cs index ac91e3c0c7..7cb19a6d49 100644 --- a/iothub/service/src/Messaging/MessagesClientError.cs +++ b/iothub/service/src/Messaging/MessagesClientError.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.IO; namespace Microsoft.Azure.Devices { @@ -9,10 +10,20 @@ namespace Microsoft.Azure.Devices /// The context provided to the error processor for a connection loss event or other failure /// when using the . /// - public class MessagesClientError : ErrorContext + public class MessagesClientError { - internal MessagesClientError(Exception exception) : base(exception) + internal MessagesClientError(Exception exception) { + Exception = exception; } + + /// + /// The exception that caused the error processor execute. + /// + /// + /// This exception is usually either of type or + /// . + /// + public Exception Exception { get; } } } diff --git a/iothub/service/src/Messaging/Models/ErrorContext.cs b/iothub/service/src/Messaging/Models/ErrorContext.cs deleted file mode 100644 index e00e3b7ff9..0000000000 --- a/iothub/service/src/Messaging/Models/ErrorContext.cs +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using System; -using System.IO; - -namespace Microsoft.Azure.Devices -{ - /// - /// The context for a given connection loss event for , - /// , and . - /// - public class ErrorContext - { - internal ErrorContext(Exception exception) - { - Exception = exception; - } - - /// - /// The exception that caused the error processor execute. - /// - /// - /// This exception is usually either of type or - /// . - /// - public Exception Exception { get; } - } -} From 8b2f80668d487953d912f8b810a09a2899a1b666 Mon Sep 17 00:00:00 2001 From: timtay-microsoft Date: Fri, 7 Apr 2023 11:23:15 -0700 Subject: [PATCH 4/4] fixup --- e2e/LongHaul/service/IotHub.cs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/e2e/LongHaul/service/IotHub.cs b/e2e/LongHaul/service/IotHub.cs index cb84c5a826..4709ac7986 100644 --- a/e2e/LongHaul/service/IotHub.cs +++ b/e2e/LongHaul/service/IotHub.cs @@ -150,12 +150,9 @@ Task OnC2dMessageAck(FeedbackBatch feedbackMessages) return Task.FromResult(AcknowledgementType.Complete); } - async Task OnC2dError(ErrorContext errorContext) + async Task OnC2dError(MessageFeedbackProcessorError error) { - Exception exToLog = errorContext.IotHubServiceException == null - ? errorContext.IOException - : errorContext.IotHubServiceException; - _logger.Trace($"Error processing C2D message.\n{exToLog}"); + _logger.Trace($"Error processing C2D message.\n{error.Exception}"); await s_serviceClient.MessageFeedback.OpenAsync(ct).ConfigureAwait(false); } @@ -205,12 +202,9 @@ async Task FileUploadNotificationCallback(FileUploadNotific return AcknowledgementType.Complete; } - async Task FileUploadNotificationErrors(ErrorContext errorContext) + async Task FileUploadNotificationErrors(FileUploadNotificationProcessorError error) { - Exception exToLog = errorContext.IotHubServiceException == null - ? errorContext.IOException - : errorContext.IotHubServiceException; - _logger.Trace($"Error processing FileUploadNotification.\n{exToLog}"); + _logger.Trace($"Error processing FileUploadNotification.\n{error.Exception}"); _logger.Trace("Attempting reconnect for FileUploadNotifications..."); await s_serviceClient.FileUploadNotifications.OpenAsync(ct).ConfigureAwait(false);