-
Notifications
You must be signed in to change notification settings - Fork 2.2k
API ServiceBusErrorSource to represent source of error #16710
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
hemanttanwar
merged 37 commits into
Azure:master
from
hemanttanwar:sb-t2-errorsource-api
Oct 30, 2020
Merged
Changes from 15 commits
Commits
Show all changes
37 commits
Select commit
Hold shift + click to select a range
b0c8562
Designing ErrorSource options
b9a83ef
Merge branch 'master' into sb-t2-errorsource-api
aba06d0
Error source structure.
28fea7b
Adding error handling for Error source
d01190a
Adding error handling for Error source
1ec13b5
Adding error handling for Error source
bd05bfe
Removed error source from SBM context
3ce0cb3
Removed unwanted changes
36106a3
Added test case for sending and receiving the messages.
1d498ae
merge master into branch
46f7d2c
Added java doc
e8d7e1f
updated module info
cbc6f55
updated code to pass error source in updateDisposition
32aae8d
module info changes will be in separate PR
e34a23e
Updated based on review comments.
d9cd7c0
changes based on review comments
fce6f55
changes based on review comments
f84f896
changes based on review comments
2d004d8
Removed Error source from Sender as dotnet is also not doing it
5a52a8d
Removed Error source from Sender as dotnet is also not doing it
578a0dd
added error source for renew lock
eda86d1
added error source for renew lock
d24f339
added error source for renew lock
6e3b5d8
added docs for unit test
c90eb0e
Renamed Exception
3286f4c
Renamed Exception
b61551f
review comments incorporated
204fd28
Fixing checkstyle
47bfc0b
Incorporated review comments
e62f7f5
Review comments
4c11eb4
Review comments
59ff15f
Review comments
5b0d16a
Removed unwanted error source types
205fbfe
Fix unit test and ErrorSource only for AutoComplete on
62510bd
Review Comments
6d79df0
Fix unit test
0a6fea9
merge conflict resolution
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
27 changes: 27 additions & 0 deletions
27
...saging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusErrorSource.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package com.azure.messaging.servicebus; | ||
|
|
||
| /** | ||
| * Represent the operation user was performing when the error happened. | ||
| */ | ||
| public enum ServiceBusErrorSource { | ||
|
hemanttanwar marked this conversation as resolved.
Outdated
|
||
| /** Error while sending the message(s).*/ | ||
| SEND, | ||
|
hemanttanwar marked this conversation as resolved.
Outdated
|
||
|
|
||
| /** Error while receiving the message(s).*/ | ||
| RECEIVE, | ||
|
|
||
| /** Error while abandoning the message.*/ | ||
| ABANDONED, | ||
|
|
||
| /** Error while completing the message.*/ | ||
| COMPLETE, | ||
|
|
||
| /** Error while deferring the message.*/ | ||
| DEFER, | ||
|
|
||
| /** Error while dead-lettering the message.*/ | ||
| DEAD_LETTER; | ||
| } | ||
35 changes: 35 additions & 0 deletions
35
...essaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusException.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package com.azure.messaging.servicebus; | ||
|
|
||
| import com.azure.core.amqp.exception.AmqpException; | ||
|
|
||
| /** | ||
| * Defines {@link ServiceBusException} which has addition properties. You can {@link ServiceBusErrorSource} to | ||
| * determine source of error. | ||
| * | ||
| * @see ServiceBusErrorSource | ||
| */ | ||
| public class ServiceBusException extends AmqpException { | ||
|
hemanttanwar marked this conversation as resolved.
Outdated
|
||
| private final ServiceBusErrorSource errorSource; | ||
|
|
||
| /** | ||
| * @param amqpException for the error hapened. | ||
| * @param errorSource indicating which api caused the error. | ||
| */ | ||
| ServiceBusException(AmqpException amqpException, ServiceBusErrorSource errorSource) { | ||
| super(amqpException.isTransient(), amqpException.getErrorCondition(), amqpException.getMessage(), | ||
| amqpException.getCause(), amqpException.getContext()); | ||
| this.errorSource = errorSource; | ||
| } | ||
|
|
||
| /** | ||
| * Gets the {@link ServiceBusErrorSource} in case of any errors. | ||
| * | ||
| * @return the {@link ServiceBusErrorSource} | ||
| */ | ||
| public ServiceBusErrorSource getErrorSource() { | ||
| return errorSource; | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ | |
| import com.azure.core.amqp.AmqpRetryOptions; | ||
| import com.azure.core.amqp.AmqpTransportType; | ||
| import com.azure.core.amqp.ProxyOptions; | ||
| import com.azure.core.amqp.exception.AmqpException; | ||
| import com.azure.core.amqp.implementation.CbsAuthorizationType; | ||
| import com.azure.core.amqp.implementation.ConnectionOptions; | ||
| import com.azure.core.amqp.implementation.MessageSerializer; | ||
|
|
@@ -29,6 +30,7 @@ | |
| import com.azure.messaging.servicebus.models.ReceiveMode; | ||
| import org.apache.qpid.proton.amqp.messaging.Accepted; | ||
| import org.apache.qpid.proton.amqp.messaging.Rejected; | ||
| import org.apache.qpid.proton.amqp.transport.DeliveryState; | ||
| import org.apache.qpid.proton.amqp.transport.DeliveryState.DeliveryStateType; | ||
| import org.apache.qpid.proton.engine.SslDomain; | ||
| import org.apache.qpid.proton.message.Message; | ||
|
|
@@ -40,7 +42,9 @@ | |
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.TestInfo; | ||
| import org.junit.jupiter.params.ParameterizedTest; | ||
| import org.junit.jupiter.params.provider.Arguments; | ||
| import org.junit.jupiter.params.provider.EnumSource; | ||
| import org.junit.jupiter.params.provider.MethodSource; | ||
| import org.mockito.ArgumentCaptor; | ||
| import org.mockito.Mock; | ||
| import org.mockito.Mockito; | ||
|
|
@@ -65,6 +69,7 @@ | |
| import java.util.UUID; | ||
| import java.util.stream.Collectors; | ||
| import java.util.stream.IntStream; | ||
| import java.util.stream.Stream; | ||
|
|
||
| import static com.azure.messaging.servicebus.TestUtils.getMessage; | ||
| import static java.nio.charset.StandardCharsets.UTF_8; | ||
|
|
@@ -443,6 +448,92 @@ void deadLetterWithDescription() { | |
| verify(amqpReceiveLink).updateDisposition(eq(lockToken1), isA(Rejected.class)); | ||
| } | ||
|
|
||
| /** | ||
| * Verifies that error source is populated when there is any error during message settlement. | ||
| */ | ||
| @ParameterizedTest | ||
| @MethodSource | ||
| void errorSourceOnSettlement(DispositionStatus dispositionStatus, ServiceBusErrorSource expectedErrorSource, | ||
| DeliveryStateType expectedDeliveryState) { | ||
| final String lockToken1 = UUID.randomUUID().toString(); | ||
|
|
||
| final OffsetDateTime expiration = OffsetDateTime.now().plus(Duration.ofMinutes(5)); | ||
|
|
||
| final MessageWithLockToken message = mock(MessageWithLockToken.class); | ||
|
|
||
| when(messageSerializer.deserialize(message, ServiceBusReceivedMessage.class)).thenReturn(receivedMessage); | ||
|
|
||
| when(receivedMessage.getLockToken()).thenReturn(lockToken1); | ||
| when(receivedMessage.getLockedUntil()).thenReturn(expiration); | ||
|
|
||
| when(amqpReceiveLink.updateDisposition(eq(lockToken1), argThat(e -> e.getType() == expectedDeliveryState))) | ||
| .thenReturn(Mono.error(new AmqpException(false, "some error occured.", null))); | ||
|
|
||
| // Act & Assert | ||
| StepVerifier.create(receiver.receiveMessages().take(1) | ||
| .flatMap(context -> { | ||
| final Mono<Void> operation; | ||
| switch (dispositionStatus) { | ||
| case DEFERRED: | ||
| operation = receiver.defer(receivedMessage); | ||
| break; | ||
| case ABANDONED: | ||
| operation = receiver.abandon(receivedMessage); | ||
| break; | ||
| case COMPLETED: | ||
| operation = receiver.complete(receivedMessage); | ||
| break; | ||
| case SUSPENDED: | ||
| operation = receiver.deadLetter(receivedMessage); | ||
| break; | ||
| default: | ||
| throw new IllegalArgumentException("Unrecognized operation: " + dispositionStatus); | ||
| } | ||
| return operation; | ||
| })) | ||
| .then(() -> messageSink.next(message)) | ||
| .expectNext() | ||
| .verifyErrorMatches(throwable -> { | ||
|
hemanttanwar marked this conversation as resolved.
Outdated
|
||
| Assertions.assertTrue(throwable instanceof ServiceBusException); | ||
| final ServiceBusErrorSource actual = ((ServiceBusException) throwable).getErrorSource(); | ||
| Assertions.assertEquals(expectedErrorSource, actual); | ||
| return true; | ||
| }); | ||
|
|
||
| verify(amqpReceiveLink).updateDisposition(eq(lockToken1), any(DeliveryState.class)); | ||
| } | ||
|
|
||
| /** | ||
| * Verifies that error source is populated when there is any error during receiving of message. | ||
| */ | ||
| @Test | ||
| void errorSourceOnReceiveMessage() { | ||
| final String lockToken1 = UUID.randomUUID().toString(); | ||
|
hemanttanwar marked this conversation as resolved.
Outdated
|
||
|
|
||
| final OffsetDateTime expiration = OffsetDateTime.now().plus(Duration.ofMinutes(5)); | ||
|
|
||
| final MessageWithLockToken message = mock(MessageWithLockToken.class); | ||
|
|
||
| when(messageSerializer.deserialize(message, ServiceBusReceivedMessage.class)).thenReturn(receivedMessage); | ||
|
|
||
| when(receivedMessage.getLockToken()).thenReturn(lockToken1); | ||
| when(receivedMessage.getLockedUntil()).thenReturn(expiration); | ||
|
|
||
| when(connection.createReceiveLink(anyString(), anyString(), any(ReceiveMode.class), any(), | ||
| any(MessagingEntityType.class))).thenReturn(Mono.error(new AmqpException(false, "some receive link Error.", null))); | ||
|
|
||
| // Act & Assert | ||
| StepVerifier.create(receiver.receiveMessages().take(1)) | ||
| .verifyErrorMatches(throwable -> { | ||
| Assertions.assertTrue(throwable instanceof ServiceBusException); | ||
| final ServiceBusErrorSource actual = ((ServiceBusException) throwable).getErrorSource(); | ||
| Assertions.assertEquals(ServiceBusErrorSource.RECEIVE, actual); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we also have tests for error source |
||
| return true; | ||
| }); | ||
|
|
||
| verify(amqpReceiveLink, never()).updateDisposition(eq(lockToken1), any(DeliveryState.class)); | ||
| } | ||
|
|
||
| /** | ||
| * Verifies that the user can complete settlement methods on received message. | ||
| */ | ||
|
|
@@ -970,4 +1061,12 @@ private List<Message> getMessages() { | |
| .mapToObj(index -> getMessage(PAYLOAD_BYTES, messageTrackingUUID, map)) | ||
| .collect(Collectors.toList()); | ||
| } | ||
|
|
||
| private static Stream<Arguments> errorSourceOnSettlement() { | ||
| return Stream.of( | ||
| Arguments.of(DispositionStatus.DEFERRED, ServiceBusErrorSource.DEFER, DeliveryStateType.Modified), | ||
| Arguments.of(DispositionStatus.COMPLETED, ServiceBusErrorSource.COMPLETE, DeliveryStateType.Accepted), | ||
| Arguments.of(DispositionStatus.SUSPENDED, ServiceBusErrorSource.DEAD_LETTER, DeliveryStateType.Rejected), | ||
| Arguments.of(DispositionStatus.ABANDONED, ServiceBusErrorSource.ABANDONED, DeliveryStateType.Modified)); | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.