Skip to content

Commit

Permalink
ClientClosed exception reason removal (Azure#13254)
Browse files Browse the repository at this point in the history
* Removed ClientClosed as a type of ServiceBusException. Replaced the instance of ClientClosed with a ObjectDisposed exception.

* Removed AssertNotClosed since it served the same purpose as AssertNotDisposed.

* Export API update.
  • Loading branch information
MiYanni authored Jul 6, 2020
1 parent 257b94a commit a238746
Show file tree
Hide file tree
Showing 9 changed files with 38 additions and 64 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -130,20 +130,19 @@ public ServiceBusException(string message, Azure.Messaging.ServiceBus.ServiceBus
public enum FailureReason
{
GeneralError = 0,
ClientClosed = 1,
MessagingEntityNotFound = 2,
MessageLockLost = 3,
MessageNotFound = 4,
MessageSizeExceeded = 5,
MessagingEntityDisabled = 6,
QuotaExceeded = 7,
ServiceBusy = 8,
ServiceTimeout = 9,
ServiceCommunicationProblem = 10,
SessionCannotBeLocked = 11,
SessionLockLost = 12,
Unauthorized = 13,
MessagingEntityAlreadyExists = 14,
MessagingEntityNotFound = 1,
MessageLockLost = 2,
MessageNotFound = 3,
MessageSizeExceeded = 4,
MessagingEntityDisabled = 5,
QuotaExceeded = 6,
ServiceBusy = 7,
ServiceTimeout = 8,
ServiceCommunicationProblem = 9,
SessionCannotBeLocked = 10,
SessionLockLost = 11,
Unauthorized = 12,
MessagingEntityAlreadyExists = 13,
}
}
public partial class ServiceBusMessage
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public override TransportSender CreateSender(
ServiceBusRetryPolicy retryPolicy,
string identifier)
{
Argument.AssertNotClosed(_closed, nameof(AmqpClient));
Argument.AssertNotDisposed(_closed, nameof(AmqpClient));

return new AmqpSender
(
Expand Down Expand Up @@ -156,7 +156,7 @@ public override TransportReceiver CreateReceiver(
string sessionId,
bool isSessionReceiver)
{
Argument.AssertNotClosed(_closed, nameof(AmqpClient));
Argument.AssertNotDisposed(_closed, nameof(AmqpClient));

return new AmqpReceiver
(
Expand Down Expand Up @@ -186,7 +186,7 @@ public override TransportRuleManager CreateRuleManager(
ServiceBusRetryPolicy retryPolicy,
string identifier)
{
Argument.AssertNotClosed(_closed, nameof(AmqpClient));
Argument.AssertNotDisposed(_closed, nameof(AmqpClient));

return new AmqpRuleManager
(
Expand Down
19 changes: 0 additions & 19 deletions sdk/servicebus/Azure.Messaging.ServiceBus/src/Core/Argument.cs
Original file line number Diff line number Diff line change
Expand Up @@ -134,25 +134,6 @@ public static void AssertNotDisposed(bool wasDisposed, string targetName)
}
}

/// <summary>
/// Ensures that an instance has not been closed, throwing an
/// <see cref="ServiceBusException" /> if that invariant is not met.
/// </summary>
///
/// <param name="wasClosed"><c>true</c> if the target instance has been closed; otherwise, <c>false</c>.</param>
/// <param name="targetName">The name of the target instance that is being verified.</param>
///
public static void AssertNotClosed(bool wasClosed, string targetName)
{
if (wasClosed)
{
throw new ServiceBusException(
string.Format(CultureInfo.CurrentCulture, Resources.ClosedInstanceCannotPerformOperation, targetName),
ServiceBusException.FailureReason.ClientClosed,
targetName);
}
}

/// <summary>
/// Ensures that an argument's value is a well-formed Service Bus fully qualified namespace value,
/// throwing a <see cref="ArgumentException" /> if that invariant is not met.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,6 @@ public enum FailureReason
/// <summary>The exception was the result of a general error within the client library.</summary>
GeneralError,

/// <summary>An operation has been attempted using an Service Bus client instance
/// which has already been closed.
/// </summary>
ClientClosed,

/// <summary>A Service Bus resource cannot be found by the Service Bus service.</summary>
MessagingEntityNotFound,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ public virtual async Task<IReadOnlyList<ServiceBusReceivedMessage>> ReceiveMessa
CancellationToken cancellationToken = default)
{
Argument.AssertAtLeast(maxMessages, 1, nameof(maxMessages));
Argument.AssertNotClosed(IsDisposed, nameof(ServiceBusReceiver));
Argument.AssertNotDisposed(IsDisposed, nameof(ServiceBusReceiver));
if (maxWaitTime.HasValue)
{
Argument.AssertPositive(maxWaitTime.Value, nameof(maxWaitTime));
Expand Down Expand Up @@ -375,7 +375,7 @@ private async Task<IReadOnlyList<ServiceBusReceivedMessage>> PeekMessagesInterna
int maxMessages,
CancellationToken cancellationToken)
{
Argument.AssertNotClosed(IsDisposed, nameof(ServiceBusReceiver));
Argument.AssertNotDisposed(IsDisposed, nameof(ServiceBusReceiver));
cancellationToken.ThrowIfCancellationRequested<TaskCanceledException>();
Logger.PeekMessageStart(Identifier, sequenceNumber, maxMessages);
using DiagnosticScope scope = ScopeFactory.CreateScope(
Expand Down Expand Up @@ -452,7 +452,7 @@ public virtual async Task CompleteMessageAsync(
CancellationToken cancellationToken = default)
{
ThrowIfLockTokenIsEmpty(lockToken);
Argument.AssertNotClosed(IsDisposed, nameof(ServiceBusReceiver));
Argument.AssertNotDisposed(IsDisposed, nameof(ServiceBusReceiver));
Argument.AssertNotNullOrEmpty(lockToken, nameof(lockToken));
ThrowIfNotPeekLockMode();
cancellationToken.ThrowIfCancellationRequested<TaskCanceledException>();
Expand Down Expand Up @@ -529,7 +529,7 @@ public virtual async Task AbandonMessageAsync(
CancellationToken cancellationToken = default)
{
ThrowIfLockTokenIsEmpty(lockToken);
Argument.AssertNotClosed(IsDisposed, nameof(ServiceBusReceiver));
Argument.AssertNotDisposed(IsDisposed, nameof(ServiceBusReceiver));
ThrowIfNotPeekLockMode();
cancellationToken.ThrowIfCancellationRequested<TaskCanceledException>();
Logger.AbandonMessageStart(Identifier, 1, lockToken);
Expand Down Expand Up @@ -688,7 +688,7 @@ private async Task DeadLetterInternalAsync(
CancellationToken cancellationToken = default)
{
ThrowIfLockTokenIsEmpty(lockToken);
Argument.AssertNotClosed(IsDisposed, nameof(ServiceBusReceiver));
Argument.AssertNotDisposed(IsDisposed, nameof(ServiceBusReceiver));
cancellationToken.ThrowIfCancellationRequested<TaskCanceledException>();
ThrowIfNotPeekLockMode();
Logger.DeadLetterMessageStart(Identifier, 1, lockToken);
Expand Down Expand Up @@ -768,7 +768,7 @@ public virtual async Task DeferMessageAsync(
CancellationToken cancellationToken = default)
{
ThrowIfLockTokenIsEmpty(lockToken);
Argument.AssertNotClosed(IsDisposed, nameof(ServiceBusReceiver));
Argument.AssertNotDisposed(IsDisposed, nameof(ServiceBusReceiver));
ThrowIfNotPeekLockMode();
cancellationToken.ThrowIfCancellationRequested<TaskCanceledException>();
Logger.DeferMessageStart(Identifier, 1, lockToken);
Expand Down Expand Up @@ -848,7 +848,7 @@ public virtual async Task<IReadOnlyList<ServiceBusReceivedMessage>> ReceiveDefer
IEnumerable<long> sequenceNumbers,
CancellationToken cancellationToken = default)
{
Argument.AssertNotClosed(IsDisposed, nameof(ServiceBusReceiver));
Argument.AssertNotDisposed(IsDisposed, nameof(ServiceBusReceiver));
Argument.AssertNotNullOrEmpty(sequenceNumbers, nameof(sequenceNumbers));
cancellationToken.ThrowIfCancellationRequested<TaskCanceledException>();
var sequenceNumbersList = sequenceNumbers.ToList();
Expand Down Expand Up @@ -921,7 +921,7 @@ public virtual async Task<DateTimeOffset> RenewMessageLockAsync(
CancellationToken cancellationToken = default)
{
ThrowIfLockTokenIsEmpty(lockToken);
Argument.AssertNotClosed(IsDisposed, nameof(ServiceBusReceiver));
Argument.AssertNotDisposed(IsDisposed, nameof(ServiceBusReceiver));
ThrowIfNotPeekLockMode();
ThrowIfSessionReceiver();
cancellationToken.ThrowIfCancellationRequested<TaskCanceledException>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ protected ServiceBusSessionReceiver() : base() { }
/// <returns>The session state as byte array.</returns>
public virtual async Task<byte[]> GetSessionStateAsync(CancellationToken cancellationToken = default)
{
Argument.AssertNotClosed(IsDisposed, nameof(ServiceBusSessionReceiver));
Argument.AssertNotDisposed(IsDisposed, nameof(ServiceBusSessionReceiver));
cancellationToken.ThrowIfCancellationRequested<TaskCanceledException>();
Logger.GetSessionStateStart(Identifier, SessionId);
using DiagnosticScope scope = ScopeFactory.CreateScope(
Expand Down Expand Up @@ -137,7 +137,7 @@ public virtual async Task SetSessionStateAsync(
byte[] sessionState,
CancellationToken cancellationToken = default)
{
Argument.AssertNotClosed(IsDisposed, nameof(ServiceBusSessionReceiver));
Argument.AssertNotDisposed(IsDisposed, nameof(ServiceBusSessionReceiver));
cancellationToken.ThrowIfCancellationRequested<TaskCanceledException>();
Logger.SetSessionStateStart(Identifier, SessionId);
using DiagnosticScope scope = ScopeFactory.CreateScope(
Expand Down Expand Up @@ -177,7 +177,7 @@ public virtual async Task SetSessionStateAsync(
/// </remarks>
public virtual async Task RenewSessionLockAsync(CancellationToken cancellationToken = default)
{
Argument.AssertNotClosed(IsDisposed, nameof(ServiceBusSessionReceiver));
Argument.AssertNotDisposed(IsDisposed, nameof(ServiceBusSessionReceiver));
cancellationToken.ThrowIfCancellationRequested<TaskCanceledException>();
Logger.RenewSessionLockStart(Identifier, SessionId);
using DiagnosticScope scope = ScopeFactory.CreateScope(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public virtual async Task AddRuleAsync(
RuleDescription description,
CancellationToken cancellationToken = default)
{
Argument.AssertNotClosed(IsDisposed, nameof(ServiceBusRuleManager));
Argument.AssertNotDisposed(IsDisposed, nameof(ServiceBusRuleManager));
Argument.AssertNotNull(description, nameof(description));
cancellationToken.ThrowIfCancellationRequested<TaskCanceledException>();
EntityNameFormatter.CheckValidRuleName(description.Name);
Expand Down Expand Up @@ -154,7 +154,7 @@ public virtual async Task RemoveRuleAsync(
string ruleName,
CancellationToken cancellationToken = default)
{
Argument.AssertNotClosed(IsDisposed, nameof(ServiceBusRuleManager));
Argument.AssertNotDisposed(IsDisposed, nameof(ServiceBusRuleManager));
Argument.AssertNotNullOrEmpty(ruleName, nameof(ruleName));
cancellationToken.ThrowIfCancellationRequested<TaskCanceledException>();
ServiceBusEventSource.Log.RemoveRuleStart(Identifier, ruleName);
Expand Down Expand Up @@ -185,7 +185,7 @@ await InnerRuleManager.RemoveRuleAsync(
public virtual async Task<IList<RuleDescription>> GetRulesAsync(CancellationToken cancellationToken = default)
{

Argument.AssertNotClosed(IsDisposed, nameof(ServiceBusRuleManager));
Argument.AssertNotDisposed(IsDisposed, nameof(ServiceBusRuleManager));
cancellationToken.ThrowIfCancellationRequested<TaskCanceledException>();
ServiceBusEventSource.Log.GetRuleStart(Identifier);
IList<RuleDescription> rulesDescription;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ public virtual async Task SendMessagesAsync(
CancellationToken cancellationToken = default)
{
Argument.AssertNotNull(messages, nameof(messages));
Argument.AssertNotClosed(IsDisposed, nameof(ServiceBusSender));
Argument.AssertNotDisposed(IsDisposed, nameof(ServiceBusSender));
IList<ServiceBusMessage> messageList = messages.ToList();
if (messageList.Count == 0)
{
Expand Down Expand Up @@ -308,7 +308,7 @@ public virtual async ValueTask<ServiceBusMessageBatch> CreateMessageBatchAsync(
CreateMessageBatchOptions options,
CancellationToken cancellationToken = default)
{
Argument.AssertNotClosed(IsDisposed, nameof(ServiceBusSender));
Argument.AssertNotDisposed(IsDisposed, nameof(ServiceBusSender));
options = options?.Clone() ?? new CreateMessageBatchOptions();
cancellationToken.ThrowIfCancellationRequested<TaskCanceledException>();
Logger.CreateMessageBatchStart(Identifier);
Expand Down Expand Up @@ -343,7 +343,7 @@ public virtual async Task SendMessagesAsync(
CancellationToken cancellationToken = default)
{
Argument.AssertNotNull(messageBatch, nameof(messageBatch));
Argument.AssertNotClosed(IsDisposed, nameof(ServiceBusSender));
Argument.AssertNotDisposed(IsDisposed, nameof(ServiceBusSender));
cancellationToken.ThrowIfCancellationRequested<TaskCanceledException>();
Logger.SendMessageStart(Identifier, messageBatch.Count);
using DiagnosticScope scope = CreateDiagnosticScope(
Expand Down Expand Up @@ -425,7 +425,7 @@ public virtual async Task<long[]> ScheduleMessagesAsync(
CancellationToken cancellationToken = default)
{
Argument.AssertNotNullOrEmpty(messages, nameof(messages));
Argument.AssertNotClosed(IsDisposed, nameof(ServiceBusSender));
Argument.AssertNotDisposed(IsDisposed, nameof(ServiceBusSender));
cancellationToken.ThrowIfCancellationRequested<TaskCanceledException>();
var messageList = messages.ToList();
await ApplyPlugins(messageList).ConfigureAwait(false);
Expand Down Expand Up @@ -483,7 +483,7 @@ public virtual async Task CancelScheduledMessagesAsync(
IEnumerable<long> sequenceNumbers,
CancellationToken cancellationToken = default)
{
Argument.AssertNotClosed(IsDisposed, nameof(ServiceBusSender));
Argument.AssertNotDisposed(IsDisposed, nameof(ServiceBusSender));
cancellationToken.ThrowIfCancellationRequested<TaskCanceledException>();
var sequenceNumberList = sequenceNumbers.ToArray();
Logger.CancelScheduledMessagesStart(Identifier, sequenceNumberList);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,10 +223,9 @@ public async Task CloseSenderShouldNotCloseConnection()
var sequenceNum = await sender.ScheduleMessageAsync(GetMessage(), scheduleTime);
await sender.DisposeAsync(); // shouldn't close connection, but should close send link

Assert.That(async () => await sender.SendMessageAsync(GetMessage()),
Throws.InstanceOf<ServiceBusException>().And.Property(nameof(ServiceBusException.Reason)).EqualTo(ServiceBusException.FailureReason.ClientClosed));
Assert.That(async () => await sender.ScheduleMessageAsync(GetMessage(), default), Throws.InstanceOf<ServiceBusException>().And.Property(nameof(ServiceBusException.Reason)).EqualTo(ServiceBusException.FailureReason.ClientClosed));
Assert.That(async () => await sender.CancelScheduledMessageAsync(sequenceNum), Throws.InstanceOf<ServiceBusException>().And.Property(nameof(ServiceBusException.Reason)).EqualTo(ServiceBusException.FailureReason.ClientClosed));
Assert.That(async () => await sender.SendMessageAsync(GetMessage()), Throws.InstanceOf<ObjectDisposedException>());
Assert.That(async () => await sender.ScheduleMessageAsync(GetMessage(), default), Throws.InstanceOf<ObjectDisposedException>());
Assert.That(async () => await sender.CancelScheduledMessageAsync(sequenceNum), Throws.InstanceOf<ObjectDisposedException>());

// receive should still work
await using var receiver = client.CreateReceiver(scope.QueueName);
Expand Down

0 comments on commit a238746

Please sign in to comment.