spring-integration icon indicating copy to clipboard operation
spring-integration copied to clipboard

Revise all the throws Exception; // NOSONAR

Open artembilan opened this issue 5 years ago • 7 comments

For example we have a code in the WebSocketListener like this:

void onMessage(WebSocketSession session, WebSocketMessage<?> message)
			throws Exception; // NOSONAR Remove in 5.2

so, all those throws Exception have to be removed.

artembilan avatar Nov 05 '20 17:11 artembilan

is this for every class in the entire codebase? any guidelines on how to revise these throw exceptions?

askzy avatar Apr 13 '21 05:04 askzy

I just did a search for this sentence: throws Exception; // NOSONAR:

image

So, not too much.

The goal is to not have such a generic throws Exception; at all. The revision indeed depends on the impl of such a contract: sometimes it is going to work as is because the impl already does a good job for error handling or it just re-throw a RuntimeException which doesn't require throws Exception; contract.

In other places we would need to what exceptions are thrown and re-throw them as some particular RuntimeException: sometimes an IllegalStateException is enough; sometimes it has to be a MessagingException...

Let's see what is going on when you just remove them and build the project gradlew check !

artembilan avatar Apr 13 '21 13:04 artembilan

@artembilan I think I can take care of that

mipo256 avatar Mar 24 '22 21:03 mipo256

Sure!

Just pull the latest main, switch to Java 17 and go ahead with the fix!

Feel free to PR or leave comments over here if something is off or out of your control.

Thank you!

artembilan avatar Mar 24 '22 21:03 artembilan

Hello guys @Mikhail2048 are you still on this issue? If not, I would like to do it.

best regards

aml8801 avatar Jun 02 '22 09:06 aml8801

Hi @aml8801 !

You know it doesn't matter for the project who contributes the fix and since there is no any news from @Mikhail2048 for a couple months already, I think it is safe for you to take this issue and PR the fix.

Thank you!

artembilan avatar Jun 02 '22 14:06 artembilan

Hello again, Thanks for the fast reply. I applied for a few issues to work on and decided to start with another issue first. After I'm done with that one I'll be back here. If someone other want to do it earlier it ok either. best regards

aml8801 avatar Jun 06 '22 15:06 aml8801

Hi there!

We are heading to RC1 soon enough. It would be great if we incorporate the fix for this issue: it is going to be a breaking change in the API we expose. It is OK if no one takes it but me: just need to know until the end of next week.

Thank you for understanding!

artembilan avatar Oct 06 '22 20:10 artembilan

As I said before: we have release next week, so this breaking change must make it into the code base before 6.0 GA. Therefore here you are PR: https://github.com/spring-projects/spring-integration/pull/3916

artembilan avatar Oct 14 '22 20:10 artembilan