eclipse / microprofile-rest-client

MicroProfile Rest Client
Apache License 2.0
142 stars 72 forks source link

Clarify What Exceptions Can Be Thrown When Closing A Client #355

Closed anowac01 closed 10 months ago

anowac01 commented 1 year ago

Following the example in the specification making a client interface extending AutoCloseable:

public interface MyServiceClient extends AutoCloseable {

when using this interface javac gives a warning of form:

warning: [try] auto-closeable resource MyServiceClient has a member method close() that could throw InterruptedException

In theory this warning could be resolved by adding an explicit close() method with a more specific signature than the default throws Exception, as recommended by the AutoCloseable javadoc, but so far as I can see the specification is silent on what exceptions are permitted to be thrown by the close() method. It's therefore not clear how to safely resolve this warning.

There is JDK-8155591 which calls out this warning as misleading, but that bug is almost 7 years old, and it isn't clear if the solution would be to not warn or to just change the wording on the warning.

Emily-Jiang commented 1 year ago

@jim-krueger @andymc12 please provide a response for this issue.

jim-krueger commented 1 year ago

Hi, It is worth noting that this warning only shows up when doing javac -Xlint.

In addition, adding @SuppressWarnings ("try") to your interface class will eliminate this warning.

Given this, I would not be in favor of limiting implementations to a specific set of exceptions solely so users can override the close() method to throw those specific exceptions to avoid this warning.

I think having the JDK change to a better warning message is preferable here. Or to eliminate the warning entirely. It seems odd to me to place warnings like this on interface classes.

anowac01 commented 1 year ago

Thank you for the response. If the specification doesn't want to limit what exceptions can be thrown, I would suggest that the example in the specification should be updated to note the issue and that it is up to implementations to define what exceptions they throw from close().

In addition, adding @SuppressWarnings ("try") to your interface class will eliminate this warning.

Just for the benefit of anyone else experiencing this issue, this only suppresses the warning on the definition of the interface. The warning also occurs each time the interface is used in a try-with-resources, and at least in Java 11 annotating the interface does not suppress those warnings. To eliminate the warning every method/class which uses the interface needs to be annotated (or -Xlint turned off).

jim-krueger commented 1 year ago

Had not considered "try-with-resources" ramifications. Thanks for pointing that out.

Your suggestion to point out the warning in the specification and recommend that implementations define what exceptions they throw, so that users can add a close() method with those exceptions to their interface class to avoid the warning, seems reasonable to me as well.

Also, FWIW: It is not necessary to turn -Xlint off completely to avoid this warning. -Xlint:all,-try will eliminate the warning as well.

WhiteCat22 commented 10 months ago

We discussed this on today's spec call. This seems like an issue with the implementation, not the spec. We are going to close this issue.

Please re-open if this is still a concern.