Fix Sonar violations for InterruptedException handling in waitForRapidResults

[zahidblackduck]

Sonar build was failing and complaining with the following message after the pull request Enhance error message for BOM_COMPARE mode failures in rapid scan was merged,

Either re-interrupt this method or rethrow the "InterruptedException" that can be caught here.
"InterruptedException" and "ThreadDeath" should not be ignored java:S2142

This fix ensures sonar build not failing.

[Copilot]

Pull Request Overview

This PR fixes a Sonar violation related to improper InterruptedException handling in the waitForRapidResults method. The change ensures that InterruptedException is properly re-thrown rather than being caught and ignored, which addresses the static analysis tool's compliance requirements.

Key changes:

• Added explicit InterruptedException handling that re-throws the exception
• Removed unused import statement

[Copilot]

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.

Suggested change

	throw new InterruptedException(e.getMessage());
	Thread.currentThread().interrupt();
	throw new OperationException("Thread was interrupted during rapid scan wait.", e);

[andrian-sevastyanov]

This might be a good suggestion. You could simply throw e;.
If you follow what auditLog.namedInternal() does, you'll see that eventually OperationWrapper.wrapped() is called which handles InterruptedException as described here.

[zahidblackduck]

Thanks. I've updated it as per the suggestion.

[Copilot]

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

	} catch (Exception e) {
	} catch (Exception e) {
	logger.error("An unexpected error occurred during the rapid wait operation.", e);

[shantyk]

I had a pending comment on the original MR that I ultimately decided to hold back but now that we are revisiting ... Is it possible to extract the error handling logic out of OperationRunner and move it to a (possibly pre-existing) utility class? Not necessary if it's a big effort, but wondering if you see that potential also (or maybe you already tried and this was a cleaner solution) -- thanks either way!

[zahidblackduck]

I had a pending comment on the original MR that I ultimately decided to hold back but now that we are revisiting ... Is it possible to extract the error handling logic out of OperationRunner and move it to a (possibly pre-existing) utility class? Not necessary if it's a big effort, but wondering if you see that potential also (or maybe you already tried and this was a cleaner solution) -- thanks either way!

That's a great suggestion. I originally thought of this while implementing this. Are you talking about the moving the handleRapidScanException(..) method to ExceptionUtility or some similar class?
To achieve that we might need to move the specific exception handling to a kind of generic class and pass around few variables, which I wanted to avoid. Nonetheless, we can do that if that's required.
But, I wanted this merge request to be considered as a quick fix to the sonar build failure.

[andrian-sevastyanov]

I had a pending comment on the original MR that I ultimately decided to hold back but now that we are revisiting ... Is it possible to extract the error handling logic out of OperationRunner and move it to a (possibly pre-existing) utility class? Not necessary if it's a big effort, but wondering if you see that potential also (or maybe you already tried and this was a cleaner solution) -- thanks either way!

That's a great suggestion. I originally thought of this while implementing this. Are you talking about the moving the handleRapidScanException(..) method to ExceptionUtility or some similar class? To achieve that we might need to move the specific exception handling to a kind of generic class and pass around few variables, which I wanted to avoid. Nonetheless, we can do that if that's required. But, I wanted this merge request to be considered as a quick fix to the sonar build failure.

You should be able to simply let exceptions bubble up from here and they will be handled by OperationWrapper.
It seems like this is what most operations in this class do.
If we go that route, I recommend trying to simulate the exceptions and testing what the logging/behaviour is like before and after the change.

[zahidblackduck]

I had a pending comment on the original MR that I ultimately decided to hold back but now that we are revisiting ... Is it possible to extract the error handling logic out of OperationRunner and move it to a (possibly pre-existing) utility class? Not necessary if it's a big effort, but wondering if you see that potential also (or maybe you already tried and this was a cleaner solution) -- thanks either way!

That's a great suggestion. I originally thought of this while implementing this. Are you talking about the moving the handleRapidScanException(..) method to ExceptionUtility or some similar class? To achieve that we might need to move the specific exception handling to a kind of generic class and pass around few variables, which I wanted to avoid. Nonetheless, we can do that if that's required. But, I wanted this merge request to be considered as a quick fix to the sonar build failure.

You should be able to simply let exceptions bubble up from here and they will be handled by OperationWrapper. It seems like this is what most operations in this class do. If we go that route, I recommend trying to simulate the exceptions and testing what the logging/behaviour is like before and after the change.

The original intent of the ticket was to provide users with a clearer, actionable message when Rapid Scans fail with a 400 Bad Request due to the use of BOM_COMPARE_STRICT mode. Specifically, the goal was to guide users to verify that the target project version exists in the Hub, as outlined in the acceptance criteria:

Add a recommendation/tip to the user to check if project version exists in HUB when BOM_COMPARE_STRICT is used as a guidance only.

If we allow the exception to bubble up directly to OperationWrapper without intercepting it, we lose the opportunity to inspect the context when the scan was run in BOM_COMPARE_STRICT mode. This context is crucial for customizing the error message and providing meaningful guidance to the user.

In the current implementation, the exception is intercepted in waitForRapidResults(...), where we check for BOM_COMPARE_STRICT and enhance the message accordingly. The exception is then rethrown, so it still bubbles up to OperationWrapper, preserving the existing flow while improving user feedback.

So while bubbling up is generally preferred for consistency, in this case, intercepting allows us to modify the exception message without disrupting the overall exception handling mechanism.

If you can think of any cleaner way to achieve this preserving the enhanced message logic, you can suggest that.