diff --git a/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Amqp/AmqpExceptionHelper.cs b/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Amqp/AmqpExceptionHelper.cs index 11ad3313de3f..29109384137b 100644 --- a/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Amqp/AmqpExceptionHelper.cs +++ b/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Amqp/AmqpExceptionHelper.cs @@ -175,7 +175,10 @@ static Exception ToMessagingContractException(string condition, string message, return new ServiceBusException(true, message); } - public static Exception GetClientException(Exception exception, string referenceId = null, Exception innerException = null, bool connectionError = false) + // In some cases, we don't store the input exception as an inner exception of the returned exception. This means we will lose the input exception's stacktrace. One fix is to always store it. + // However, this changes the type of exception we throw, and could break clients who were depending on the exception type. Instead, we will log the input exception's stacktrace. + // This behavior will be controlled by the flag 'logExceptionIfNotWrapped', since some callers already log the input exception. + public static Exception GetClientException(Exception exception, bool logExceptionIfNotWrapped, string referenceId = null, Exception innerException = null, bool connectionError = false) { var stringBuilder = new StringBuilder(); stringBuilder.AppendFormat(CultureInfo.InvariantCulture, exception.Message); @@ -227,6 +230,11 @@ public static Exception GetClientException(Exception exception, string reference return new ServiceBusCommunicationException(message, aggregateException); } + if (aggregateException == exception && logExceptionIfNotWrapped) + { + MessagingEventSource.Log.Error($"{message}: {aggregateException}"); + } + return aggregateException; } @@ -264,7 +272,7 @@ public static Exception GetInnerException(this AmqpObject amqpObject) return null; } - return innerException == null ? null : GetClientException(innerException, null, null, connectionError); + return innerException == null ? null : GetClientException(innerException, true, null, null, connectionError); } } } \ No newline at end of file diff --git a/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Amqp/AmqpLinkCreator.cs b/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Amqp/AmqpLinkCreator.cs index f4eea4c1e48b..d442565948ba 100644 --- a/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Amqp/AmqpLinkCreator.cs +++ b/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Amqp/AmqpLinkCreator.cs @@ -75,7 +75,7 @@ public async Task> CreateAndOpenAmqpLinkAsync() { MessagingEventSource.Log.AmqpSessionCreationException(this.entityPath, amqpConnection, exception); session?.Abort(); - throw AmqpExceptionHelper.GetClientException(exception, null, session.GetInnerException(), amqpConnection.IsClosing()); + throw AmqpExceptionHelper.GetClientException(exception, false, null, session.GetInnerException(), amqpConnection.IsClosing()); } AmqpObject link = null; @@ -95,7 +95,7 @@ public async Task> CreateAndOpenAmqpLinkAsync() exception); session.SafeClose(exception); - throw AmqpExceptionHelper.GetClientException(exception, null, link?.GetInnerException(), amqpConnection.IsClosing()); + throw AmqpExceptionHelper.GetClientException(exception, false, null, link?.GetInnerException(), amqpConnection.IsClosing()); } } diff --git a/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Amqp/AmqpSubscriptionClient.cs b/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Amqp/AmqpSubscriptionClient.cs index 38e3d1029be9..1de539570fe9 100644 --- a/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Amqp/AmqpSubscriptionClient.cs +++ b/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Amqp/AmqpSubscriptionClient.cs @@ -129,7 +129,7 @@ public async Task OnAddRuleAsync(RuleDescription description) } catch (Exception exception) { - throw AmqpExceptionHelper.GetClientException(exception); + throw AmqpExceptionHelper.GetClientException(exception, true); } } @@ -153,7 +153,7 @@ public async Task OnRemoveRuleAsync(string ruleName) } catch (Exception exception) { - throw AmqpExceptionHelper.GetClientException(exception); + throw AmqpExceptionHelper.GetClientException(exception, true); } } @@ -194,7 +194,7 @@ public async Task> OnGetRulesAsync(int top, int skip) } catch (Exception exception) { - throw AmqpExceptionHelper.GetClientException(exception); + throw AmqpExceptionHelper.GetClientException(exception, true); } } } diff --git a/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Amqp/AmqpTransactionEnlistment.cs b/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Amqp/AmqpTransactionEnlistment.cs index a7223b79b59f..2301dcb21d46 100644 --- a/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Amqp/AmqpTransactionEnlistment.cs +++ b/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Amqp/AmqpTransactionEnlistment.cs @@ -73,7 +73,7 @@ async Task SinglePhaseCommitAsync(SinglePhaseEnlistment singlePhaseEnlistment) } catch (Exception e) { - Exception exception = AmqpExceptionHelper.GetClientException(e, null); + Exception exception = AmqpExceptionHelper.GetClientException(e, true, null); MessagingEventSource.Log.AmqpTransactionDischargeException(this.transactionId, this.AmqpTransactionId, exception); singlePhaseEnlistment.InDoubt(exception); } @@ -98,7 +98,7 @@ async Task RollbackAsync(SinglePhaseEnlistment singlePhaseEnlistment) } catch (Exception e) { - Exception exception = AmqpExceptionHelper.GetClientException(e, null); + Exception exception = AmqpExceptionHelper.GetClientException(e, true, null); MessagingEventSource.Log.AmqpTransactionDischargeException(this.transactionId, this.AmqpTransactionId, exception); singlePhaseEnlistment.Aborted(exception); } diff --git a/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Core/MessageReceiver.cs b/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Core/MessageReceiver.cs index 701c7273aae5..810481d40892 100644 --- a/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Core/MessageReceiver.cs +++ b/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Core/MessageReceiver.cs @@ -1124,7 +1124,7 @@ protected virtual async Task> OnReceiveAsync(int maxMessageCount, } catch (Exception exception) { - throw AmqpExceptionHelper.GetClientException(exception, receiveLink?.GetTrackingId(), null, receiveLink?.IsClosing() ?? false); + throw AmqpExceptionHelper.GetClientException(exception, true, receiveLink?.GetTrackingId(), null, receiveLink?.IsClosing() ?? false); } } @@ -1183,7 +1183,7 @@ protected virtual async Task> OnPeekAsync(long fromSequenceNumber } catch (Exception exception) { - throw AmqpExceptionHelper.GetClientException(exception); + throw AmqpExceptionHelper.GetClientException(exception, true); } } @@ -1231,7 +1231,7 @@ protected virtual async Task> OnReceiveDeferredMessageAsync(long[ } catch (Exception exception) { - throw AmqpExceptionHelper.GetClientException(exception); + throw AmqpExceptionHelper.GetClientException(exception, true); } return messages; @@ -1316,7 +1316,7 @@ protected virtual async Task OnRenewLockAsync(string lockToken) } catch (Exception exception) { - throw AmqpExceptionHelper.GetClientException(exception); + throw AmqpExceptionHelper.GetClientException(exception, true); } return lockedUntilUtc; @@ -1489,7 +1489,7 @@ async Task DisposeMessagesAsync(IEnumerable lockTokens, Outcome outcome) throw new MessageLockLostException(Resources.MessageLockLost); } - throw AmqpExceptionHelper.GetClientException(exception); + throw AmqpExceptionHelper.GetClientException(exception, true); } } @@ -1552,7 +1552,7 @@ async Task DisposeMessageRequestResponseAsync(Guid[] lockTokens, DispositionStat } catch (Exception exception) { - throw AmqpExceptionHelper.GetClientException(exception); + throw AmqpExceptionHelper.GetClientException(exception, true); } } diff --git a/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Core/MessageSender.cs b/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Core/MessageSender.cs index 21b599823c4d..d1188fa2b05c 100644 --- a/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Core/MessageSender.cs +++ b/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Core/MessageSender.cs @@ -573,7 +573,7 @@ async Task OnSendAsync(IList messageList) } catch (Exception exception) { - throw AmqpExceptionHelper.GetClientException(exception, amqpLink?.GetTrackingId(), null, amqpLink?.Session.IsClosing() ?? false); + throw AmqpExceptionHelper.GetClientException(exception, true, amqpLink?.GetTrackingId(), null, amqpLink?.Session.IsClosing() ?? false); } } } @@ -642,7 +642,7 @@ async Task OnScheduleMessageAsync(Message message) } catch (Exception exception) { - throw AmqpExceptionHelper.GetClientException(exception, sendLink?.GetTrackingId(), null, sendLink?.Session.IsClosing() ?? false); + throw AmqpExceptionHelper.GetClientException(exception, true, sendLink?.GetTrackingId(), null, sendLink?.Session.IsClosing() ?? false); } } } @@ -675,7 +675,7 @@ async Task OnCancelScheduledMessageAsync(long sequenceNumber) } catch (Exception exception) { - throw AmqpExceptionHelper.GetClientException(exception, sendLink?.GetTrackingId(), null, sendLink?.Session.IsClosing() ?? false); + throw AmqpExceptionHelper.GetClientException(exception, true, sendLink?.GetTrackingId(), null, sendLink?.Session.IsClosing() ?? false); } } diff --git a/sdk/servicebus/Microsoft.Azure.ServiceBus/src/MessageSession.cs b/sdk/servicebus/Microsoft.Azure.ServiceBus/src/MessageSession.cs index 249fa2c3e3e6..73527d950771 100644 --- a/sdk/servicebus/Microsoft.Azure.ServiceBus/src/MessageSession.cs +++ b/sdk/servicebus/Microsoft.Azure.ServiceBus/src/MessageSession.cs @@ -105,7 +105,7 @@ protected async Task OnGetStateAsync() } catch (Exception exception) { - throw AmqpExceptionHelper.GetClientException(exception); + throw AmqpExceptionHelper.GetClientException(exception, true); } } @@ -140,7 +140,7 @@ protected async Task OnSetStateAsync(byte[] sessionState) } catch (Exception exception) { - throw AmqpExceptionHelper.GetClientException(exception); + throw AmqpExceptionHelper.GetClientException(exception, true); } } @@ -170,7 +170,7 @@ protected async Task OnRenewSessionLockAsync() } catch (Exception exception) { - throw AmqpExceptionHelper.GetClientException(exception); + throw AmqpExceptionHelper.GetClientException(exception, true); } } diff --git a/sdk/servicebus/Microsoft.Azure.ServiceBus/src/MessagingEventSource.cs b/sdk/servicebus/Microsoft.Azure.ServiceBus/src/MessagingEventSource.cs index 1a2544a4c1ab..2fc20c74fdee 100644 --- a/sdk/servicebus/Microsoft.Azure.ServiceBus/src/MessagingEventSource.cs +++ b/sdk/servicebus/Microsoft.Azure.ServiceBus/src/MessagingEventSource.cs @@ -1416,6 +1416,15 @@ public void UnregisterSessionHandlerStop(string clientId) this.WriteEvent(121, clientId); } } + + [Event(122, Level = EventLevel.Error, Message = "Error occurred: {0}")] + public void Error(string errorMsg) + { + if (this.IsEnabled()) + { + this.WriteEvent(122, errorMsg); + } + } } internal static class TraceHelper diff --git a/sdk/servicebus/Microsoft.Azure.ServiceBus/src/SessionClient.cs b/sdk/servicebus/Microsoft.Azure.ServiceBus/src/SessionClient.cs index 71caabc78b1c..5f4673110f9a 100644 --- a/sdk/servicebus/Microsoft.Azure.ServiceBus/src/SessionClient.cs +++ b/sdk/servicebus/Microsoft.Azure.ServiceBus/src/SessionClient.cs @@ -331,7 +331,7 @@ public async Task AcceptMessageSessionAsync(string sessionId, T exception); await session.CloseAsync().ConfigureAwait(false); - throw AmqpExceptionHelper.GetClientException(exception); + throw AmqpExceptionHelper.GetClientException(exception, false); } finally {