Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

SecureHttpClientFactory: avoid throwing RuntimeException #5340

Closed
maggu2810 opened this issue Apr 3, 2018 · 2 comments
Closed

SecureHttpClientFactory: avoid throwing RuntimeException #5340

maggu2810 opened this issue Apr 3, 2018 · 2 comments

Comments

@maggu2810
Copy link
Contributor

maggu2810 commented Apr 3, 2018

The SecureHttpClientFactory violates the AvoidThrowingRawExceptionTypes rule.

Avoid throwing certain exception types. Rather than throw a raw RuntimeException, Throwable, Exception, or Error, use a subclassed exception or error instead.

Continues: #5318 (comment)

I assume an unchecked exception has been chosen because an interface method is implemented that does not declare any exception. So, the interface expects that the method does not fail (it also don't accept a nullable return value AFAIK).
Do we need to use an unchecked one or would it make sense to change the interface to "allow" also a non successful return?
Should we use more specific unchecked exceptions?

If this gets solved, we could remove the suppression defined in tools/static-code-analysis/pmd/suppressions.properties.

@triller-telekom
Copy link
Contributor

I would also vote for having a specific exception so the caller is aware of what happened and depending on its implementation he could react on it.

Here I think once could catch InvalidCipher, Key, etc. exceptions and throw a SecurityException.

https://github.com/eclipse/smarthome/blob/7a13dff3fd81036ddbe3d39181d4ec3fefe85883/bundles/io/org.eclipse.smarthome.io.net/src/main/java/org/eclipse/smarthome/io/net/http/internal/SecureHttpClientFactory.java#L202-L205

In

https://github.com/eclipse/smarthome/blob/7a13dff3fd81036ddbe3d39181d4ec3fefe85883/bundles/io/org.eclipse.smarthome.io.net/src/main/java/org/eclipse/smarthome/io/net/http/internal/SecureHttpClientFactory.java#L219-L224

I think we do not have another choice other than catching Exception because jetty throws it :( But nevertheless we should throw a custom exception.

For

https://github.com/eclipse/smarthome/blob/7a13dff3fd81036ddbe3d39181d4ec3fefe85883/bundles/io/org.eclipse.smarthome.io.net/src/main/java/org/eclipse/smarthome/io/net/http/internal/SecureHttpClientFactory.java#L230-L236

I think we should also create a custom exception instead of throwing RuntimeException.

@triller-telekom
Copy link
Contributor

There is PR #5409 to address this and all participants agreed on the solution.

@SJKA @kaikreuzer Can you please close this issue?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants