Conversation
mssfang
added
Service Bus
Client
| { | ||
| this.retryCounts.put(clientId, 0); | ||
| } | ||
| public void resetRetryCount(String clientId) { |
There was a problem hiding this comment.
[SpotBugs-P3]
Sequence of calls to java.util.concurrent.ConcurrentHashMap may not be atomic in com.microsoft.azure.servicebus.primitives.RetryPolicy.resetRetryCount(String)
...-servicebus/src/main/java/com/microsoft/azure/servicebus/primitives/RequestResponseLink.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| builder.append(exception.getMessage()); | ||
| if (exception.getStackTrace() != null) |
There was a problem hiding this comment.
[SpotBugs-P3]
Redundant nullcheck of Throwable.getStackTrace(), which is known to be non-null in com.microsoft.azure.servicebus.primitives.ExceptionUtil.toStackTraceString(Throwable, String)
| { | ||
| if (innerException != null) { | ||
| builder.append("Cause: " + innerException.getMessage()); | ||
| if (innerException.getStackTrace() != null) |
There was a problem hiding this comment.
[SpotBugs-P3]
Redundant nullcheck of Throwable.getStackTrace(), which is known to be non-null in com.microsoft.azure.servicebus.primitives.ExceptionUtil.toStackTraceString(Throwable, String)
| if (this.requestResponseLockTokensToLockTimesMap.containsKey(lockToken)) { | ||
| this.requestResponseLockTokensToLockTimesMap.put(lockToken, lockedUntilUtc); | ||
| } | ||
| this.requestResponseLockTokensToLockTimesMap.computeIfPresent(lockToken, (k, v) -> lockedUntilUtc); |
There was a problem hiding this comment.
[SpotBugs-P3]
Sequence of calls to java.util.concurrent.ConcurrentHashMap may not be atomic in com.microsoft.azure.servicebus.MessageReceiver.lambda$renewMessageLockBatchAsync$20(UUID[], Collection)
| public class AmqpException extends Exception { | ||
| private static final long serialVersionUID = -750417419234273714L; | ||
| private ErrorCondition errorCondition; | ||
| private transient ErrorCondition errorCondition; |
There was a problem hiding this comment.
[SpotBugs-P3]
This Serializable class defines a non-primitive instance field which is neither transient, Serializable, or java.lang.Object, and does not appear to implement the Externalizable interface or the readObject() and writeObject() methods. Objects of this class will not be deserialized correctly if a non-Serializable object is stored in this field.
There was a problem hiding this comment.
the ErrorCondition already not serializable, Ignore it with using transient modifier to suppress the SpotBugs error
...lane/azure-servicebus/src/main/java/com/microsoft/azure/servicebus/primitives/AsyncUtil.java
Outdated
Show resolved
Hide resolved
|
Can one of the admins verify this patch? |
|
|
||
| public class ReceiverErrorContext extends ErrorContext | ||
| { | ||
| private static final long serialVersionUID = -8154706630781986787L; |
There was a problem hiding this comment.
[SpotBugs]
Since ServiceBusException has serialVersionUID, it means it needs to implement Serializable. Change the ErrorContext to implements Serializable cause all its sub-extended class to have the serialVersionUID
| { | ||
| abstract class ErrorContext implements Serializable { | ||
|
|
||
| private static final long serialVersionUID = -6342329018037308640L; |
There was a problem hiding this comment.
[SpotBugs]
Since ServiceBusException has serialVersionUID, it means it needs to implement Serializable. Change the ErrorContext to implements Serializable cause all its sub-extended class to have the serialVersionUID
|
|
||
| public class SenderErrorContext extends ErrorContext | ||
| { | ||
| private static final long serialVersionUID = -8426189357575601244L; |
There was a problem hiding this comment.
[SpotBugs]
Since ServiceBusException has serialVersionUID, it means it needs to implement Serializable. Change the ErrorContext to implements Serializable cause all its sub-extended class to have the serialVersionUID
conniey
left a comment
conniey
left a comment
There was a problem hiding this comment.
Looks good. Should pass the try {} catch changes with the service team. In case they want to log those errors rather than print a stack trace.
conniey
left a comment
conniey
left a comment
There was a problem hiding this comment.
Just a comment about talking to the service team about that behaviour change.
...lane/azure-servicebus/src/main/java/com/microsoft/azure/servicebus/primitives/AsyncUtil.java
Outdated
Show resolved
Hide resolved
…into ServiceBus-P3-Two
4 participants
PR includes
(1) P3 SpotBugs: search with using [SpotBugs-P3]
(2) some checkstyles fixes to these files