-
Notifications
You must be signed in to change notification settings - Fork 80
Fix Sonar violations for InterruptedException handling in waitForRapidResults #1492
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -26,7 +26,6 @@ | |||||||
import java.util.function.Predicate; | ||||||||
import java.util.regex.Matcher; | ||||||||
import java.util.regex.Pattern; | ||||||||
import java.util.stream.Collectors; | ||||||||
|
||||||||
import com.blackduck.integration.blackduck.bdio2.model.BdioFileContent; | ||||||||
import com.blackduck.integration.detect.configuration.enumeration.RapidCompareMode; | ||||||||
|
@@ -257,7 +256,6 @@ public class OperationRunner { | |||||||
private static final String INTELLIGENT_SCAN_SCASS_CONTENT_TYPE = "application/vnd.blackducksoftware.intelligent-persistence-scan-4+protobuf-jsonld"; | ||||||||
public static final ImmutableList<Integer> RETRYABLE_AFTER_WAIT_HTTP_EXCEPTIONS = ImmutableList.of(408, 429, 502, 503, 504); | ||||||||
public static final ImmutableList<Integer> RETRYABLE_WITH_BACKOFF_HTTP_EXCEPTIONS = ImmutableList.of(425, 500); | ||||||||
private static final String POLICY_STATUS_RESOLVED = "RESOLVED"; | ||||||||
private List<File> binaryUserTargets = new ArrayList<>(); | ||||||||
BinaryScanFindMultipleTargetsOperation binaryScanFindMultipleTargetsOperation; | ||||||||
|
||||||||
|
@@ -721,10 +719,11 @@ public List<DeveloperScansScanView> waitForRapidResults(BlackDuckRunData blackDu | |||||||
mode, | ||||||||
calculateMaxWaitInSeconds(fibonacciSequenceIndex) | ||||||||
); | ||||||||
} catch (InterruptedException e) { | ||||||||
throw new InterruptedException(e.getMessage()); | ||||||||
} catch (IntegrationRestException e) { | ||||||||
throw handleRapidScanException(e); | ||||||||
} catch (Exception e) { | ||||||||
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. The removal of the error logging statement means that exceptions other than InterruptedException and IntegrationRestException will no longer be logged before being wrapped in OperationException. This could make debugging more difficult as the original exception details may be lost.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||
logger.error("Exception while waiting for rapid results: {}", e.getMessage(), e); | ||||||||
throw new OperationException(e); | ||||||||
} | ||||||||
}); | ||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating a new InterruptedException and throwing it loses the original stack trace and interrupt status. Instead, you should re-interrupt the current thread using
Thread.currentThread().interrupt()
and then rethrow the original exception or wrap it in an unchecked exception.Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be a good suggestion. You could simply
throw e;
.If you follow what
auditLog.namedInternal()
does, you'll see that eventuallyOperationWrapper.wrapped()
is called which handlesInterruptedException
as described here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I've updated it as per the suggestion.