[ https://issues.apache.org/jira/browse/SLING-7510?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Radu Cotescu updated SLING-7510: -------------------------------- Fix Version/s: (was: API 2.22.0) API 2.22.2 > UriProvider throws unchecked IllegalArgumentException that must be handled by > consumers > --------------------------------------------------------------------------------------- > > Key: SLING-7510 > URL: https://issues.apache.org/jira/browse/SLING-7510 > Project: Sling > Issue Type: Improvement > Components: API > Affects Versions: API 2.16.4 > Reporter: Alexander Klimetschek > Priority: Major > Fix For: API 2.22.2 > > > h3. Status quo > A consumer of the > [UriProvider|https://github.com/apache/sling-org-apache-sling-api/blob/dfc41640031bc87ec271c648b22073e65f4f171a/src/main/java/org/apache/sling/api/resource/external/URIProvider.java#L45] > currently is required to handle an unchecked {{IllegalArgumentException}}, > which is thrown when the provider is not able to handle the binary. Note that > it is not supposed to ever return null per the javadoc. The > [JcrNodeResource|https://github.com/apache/sling-org-apache-sling-jcr-resource/blob/0e2ebd0f1a5c7cb2044b2d754945eb0ee7641081/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrNodeResource.java#L233-L242] > shows a typical consumer code (although it still does do a null check). > For the use case of asking multiple providers and taking the first one that > responds it's not an optimal pattern to rely on an unchecked exception for > the expected failure case that one provider by design cannot handle a certain > binary or request. Throwing an {{IllegalArgumentException}} if there is no > problem with the argument passed from the client, but a limit or > configuration setting of the provider, is misleading. Also, given there are > multiple providers active, a client cannot know upfront which provider is the > right one for a given binary and somehow prevent the "illegal argument" call > in the first place. > h3. Suggestion > Often, {{null}} return values are used in such a case. The provider can log > any possible useful information itself, on why it could not handle it, if > needed. This would simplify the consumer code (no try/catch necessary) and > remove unnecessary cost of exception handling for normal code paths. > JcrNodeResource itself it uses a null return value to pass on the "could not > retrieve anything" state to the upper layers. > If the goal really is to use exceptions here, the API should add a > {{@Nonnull}} annotation for the return value _and_ the expected failure > exception should be a checked one such as a new {{UriProviderException}}. > Then for any unexpected faults (e.g. network error), it's fine to allow > providers to throw a unchecked runtime exception, and usually that's not > something that is explicitly mentioned in javadoc, but would definitely not > hurt. -- This message was sent by Atlassian Jira (v8.3.4#803005)