[
https://issues.apache.org/jira/browse/SLING-7510?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Alexander Klimetschek updated SLING-7510:
-----------------------------------------
Summary: UriProvider throws unchecked IllegalArgumentException that must be
handled by consumers (was: UriProvider should throw a checked exception
instead of IllegalArgumentException)
> 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
>
> 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
(v7.6.3#76005)
