directory-api mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Emmanuel Lécharny <elecha...@gmail.com>
Subject Re: Maybe Cursor should also extends Closable
Date Tue, 02 Feb 2016 10:23:46 GMT
Le 02/02/16 10:52, Radovan Semancik a écrit :
> +1 for going the Closeable way.
>
> Close operation can end up with an error (e.g. timeout). That cannot
> be avoided. And for me it is much easier (and much less error-prone)
> to handle a checked exception than to do a complex sequence of close()
> and then check for isClosed() or isConnected() and similar and then
> trying to figure out what really was the root cause because the
> original exception got eaten somewhere deep in the code. Having
> close() that throws IOException will be a nudge to all the clients to
> do at least some error handling.
>
> Of course, the problem is that it is IOException and not
> LdapException. One of the solution would be to make LdapException
> subclass of IOException? That obviously does not fit 100% well, as
> some subclasses of LdapException and more related to parsing than to
> IO. But I think that it is still somehow acceptable. Another (and
> perhaps cleaner) would be to throw IO exceptions that happen during
> the close directly as IO exceptions without wrapping them to
> LdapException. And that might be the right thing to also for all other
> operations: if the operation dies on network condition (e.g. TCP
> timeout), throw original IO exception. If it dies on LDAP condition
> (e.g. LDAP error code 49) throw LdapException. That would solve the
> connection being Closeable. Now for cursor there is still this tricky
> part that the close() on cursor might want to abort an operation and
> therefore it may produce LDAP errors. So maybe the best thing to do
> would to do both: make LdapException subclass of IOException and pass
> low-level IO exceptions without wrapping them to LdapException.
>
> This is how I would do it. But maybe the API has some other philosophy
> when dealing with errors? If there is one, I must have missed it so I
> will be very grateful for anyone to point my nose in the right
> direction. However, I really think one of the big drawbacks of the API
> is error handling in general. Especially in the schema processing
> part, but there are obviously also other parts that could use some
> improvement. We obviously cannot change everything in a day. So
> gradually improving the status quo seems to be a good way to take.
>
FTR, I have changed the Cursor interface to extends Closeable. After a
few changes, I tested teh API and ApacheDS, it does the job.


It makes no sense for the Cursor close() method to throw a
LdapException, it's way to narrow. Throwing an IoException is just fine.

regarding the exception handling in general, in teh API, yes, we are
aware of it. We have a JIRA opened since 2011 :
https://issues.apache.org/jira/browse/DIRAPI-116?jql=project%20%3D%20DIRAPI%20AND%20resolution%20%3D%20Unresolved%20ORDER%20BY%20priority%20DESC.
This is not an easy thing to fix though, as it impacts a hell lot of
classes, but at least we can start to think about it.

I'd suggest to create another thread to talk about this aspect.

In the mean time, I'm going to commit the Cursor change (still have to
check the impact on Studio).

thanks !

Mime
View raw message