Skip to content
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

Methods in org.jboss.eap.qe.ts.common.docker.Docker tool mask exceptions #194

Open
honza-kasik opened this issue Jan 22, 2020 · 3 comments
Assignees

Comments

@honza-kasik
Copy link
Member

Almost every method in org.jboss.eap.qe.ts.common.docker.Docker has throws Exception in its signature which causes masking of exception and leaves no precise information what kind of exception method is throwing.

@mnovak1
Copy link
Member

mnovak1 commented Jan 23, 2020

afaict when any specific exception is thrown then it's not generalized as Exception, user will always see the specific exception. Only issue is with method signatures where the motivation to throw general Exception was to keep it simple avoid API breakage in the future. Or do I understand it wrong?

@honza-kasik
Copy link
Member Author

It is a bad for anyone who wants to expand current methods. IDE will not remind him that an exception is thrown by newly added code which he might want to catch. It is also bad for anyone who want to use these methods in his code because using them effectively masks any other exception in code and then it is similar to first case. Using throws Exception makes risk of introducing an error to the code much higher.

We can use checked exceptions instead. Such as "ContainerStartException" which will be the only exception thrown by the start method to avoid API break in future and not masking other exceptions.

@mnovak1
Copy link
Member

mnovak1 commented Jan 23, 2020

We can use checked exceptions instead. Such as "ContainerStartException" which will be the only exception thrown by the start method to avoid API break in future and not masking other exceptions.

Right, Exception is too generic without a way to catch anything specific. I'm ok with this change.

honza-kasik added a commit to honza-kasik/eap-microprofile-test-suite that referenced this issue Jan 23, 2020
honza-kasik added a commit to honza-kasik/eap-microprofile-test-suite that referenced this issue Jan 23, 2020
honza-kasik added a commit to honza-kasik/eap-microprofile-test-suite that referenced this issue Jan 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants