You are viewing a plain text version of this content. The canonical link for it is here.
Posted to api@directory.apache.org by Emmanuel Lecharny <el...@gmail.com> on 2010/07/19 16:49:20 UTC
About operation result
Hi,
long time, no see :)
I have a proposal about the way we handle the operation results.
Currently, every operation is returning a Response, and if we want to
know if the operation has been successful, we have to check the
LdapResult field. This is not very convenient.
Assuming that when we have issued a synchronous operation, we are
waiting until we get a response, why can't we have those operations
throwing a LdapException ?
For async operations, we can also make that the XXXFuture.get() method
throw the same exception.
thoughts ?
--
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com
Re: About operation result
Posted by Stefan Seelmann <se...@apache.org>.
On Thu, Jul 22, 2010 at 12:37 PM, Kiran Ayyagari <ka...@apache.org> wrote:
> hello guys,
>
> On Mon, Jul 19, 2010 at 10:48 PM, Emmanuel Lecharny <el...@gmail.com> wrote:
> <snip/>
>
> After a discussion on IRC we have decided to inject LdapResult object
> into the LdapException.
> Another significant choice was not to throw exceptions for the result codes
> 3(timeLimitExceeded) and 4(sizeLimitExceeded) related to the search operation.
I think we should decide per operation if an exception should be throw
for a particular result code. I already suggested a list in [1].
For the search operation I think no exception should be thrown for 3
(timeLimitExeeded) and 4 (sizeLimitExeedec) because that are
client-side limits which the user of the client API set explicitely.
But for result code 11 (adminLimitExceeded) it makes sense to throw an
exception.
> The lookup operation now returns a Entry instead of the generic
> SearchResponse and throws
> ReferralException incase if the response received from the server is a referral.
Yes, I think that makes sense.
Kind Regards,
Stefan
[1] http://mail-archives.apache.org/mod_mbox/directory-api/201006.mbox/%3C4C05EF3B.50005@apache.org%3E
Re: About operation result
Posted by Kiran Ayyagari <ka...@apache.org>.
hello guys,
On Mon, Jul 19, 2010 at 10:48 PM, Emmanuel Lecharny <el...@gmail.com> wrote:
<snip/>
After a discussion on IRC we have decided to inject LdapResult object
into the LdapException.
Another significant choice was not to throw exceptions for the result codes
3(timeLimitExceeded) and 4(sizeLimitExceeded) related to the search operation.
The lookup operation now returns a Entry instead of the generic
SearchResponse and throws
ReferralException incase if the response received from the server is a referral.
Kiran Ayyagari
Re: About operation result
Posted by Emmanuel Lecharny <el...@gmail.com>.
On 7/19/10 7:02 PM, Matthew Swift wrote:
> Hi Emmanuel,
>
> Long time no hear!
>
> I think this is a good idea. It's what we chose to do in the OpenDS
> SDK and I think that it makes the API much more usable in practice.
> Application code is leaner and less error prone since there is no need
> to check (or forget to check!) the result code after each operation.
> Instead all error handling can be performed in a single catch block at
> the end.
Absolutly. If only you use the API for a moment, it becomes obvious that
it's the right way to go.
>
> Something else we did was to also create several subclasses of our
> ErrorResultException class in order to make it easier to isolate
> common failure reasons, e.g. connection failure, authn/authz failure,
> referral, timeout, etc:
>
> http://www.opends.org/daily-builds/sdk/latest/OpenDS_SDK/doc/org/opends/sdk/ErrorResultException.html
>
>
> I think that this is similar to JNDI. I didn't shoot for a 1:1 mapping
> between result codes and exception types since this would lead to very
> many exception classes which I thought would be a bit excessive. It's
> a trade-off, and I don't know if I set the bar too low or too high.
> The good thing is that it is possible to add more sub-classes later
> without breaking compatibility, so I erred on the low side probably.
> Since all of these exceptions expose the underlying result, it still
> possible to do a catch-all on ErrorResultException and still have
> logic based on the ResultCode.
We have done the same thing a while back :
http://mail-archives.apache.org/mod_mbox/directory-api/201003.mbox/ajax/%3C4B9A1FE4.4040909@gmail.com%3E
>
> Also note that you will need to make LdapException a sub-class of
> j.u.c.ExecutionException for it to be thrown by Future.get (or make it
> a runtime exception but I think that this is a bad idea). This is a
> bit annoying, but in practice not a big deal (it just looks surprising
> seeing java.util.concurrent in the class hierarchy for a result
> exception).
>
> Another API problem I ran into was what to do with the "checked"
> InterruptedException which can be thrown from blocking operations such
> as Future.get. I could have chosen to catch it and rethrow it as a
> cancelled result exception (or a new exception like
> InterruptedErrorResultException). This would avoid having to
> catch/throw it every time, as this example illustrates:
>
> Connection connection = ...;
> Entry entry = ...;
>
> try
> {
> connection.add(entry);
> }
> catch (ErrorResultException e)
> {
> // Handle operation failure.
> }
> catch (InterruptedException e)
> {
> // Grrr... Handle thread interrupt
>
> // This would not be needed if I caught and re-threw
> // the exception as a sub-type of ErrorResultException.
> }
>
>
> I played it safe and kept it separate since InterruptedException has a
> very specific contract, so hiding it inside an ErrorResultException
> (or LdapException in your case) might cause it to get overlooked.
+1
--
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com
Re: About operation result
Posted by Matthew Swift <ma...@oracle.com>.
Hi Emmanuel,
Long time no hear!
I think this is a good idea. It's what we chose to do in the OpenDS SDK
and I think that it makes the API much more usable in practice.
Application code is leaner and less error prone since there is no need
to check (or forget to check!) the result code after each operation.
Instead all error handling can be performed in a single catch block at
the end.
Something else we did was to also create several subclasses of our
ErrorResultException class in order to make it easier to isolate common
failure reasons, e.g. connection failure, authn/authz failure, referral,
timeout, etc:
http://www.opends.org/daily-builds/sdk/latest/OpenDS_SDK/doc/org/opends/sdk/ErrorResultException.html
I think that this is similar to JNDI. I didn't shoot for a 1:1 mapping
between result codes and exception types since this would lead to very
many exception classes which I thought would be a bit excessive. It's a
trade-off, and I don't know if I set the bar too low or too high. The
good thing is that it is possible to add more sub-classes later without
breaking compatibility, so I erred on the low side probably. Since all
of these exceptions expose the underlying result, it still possible to
do a catch-all on ErrorResultException and still have logic based on the
ResultCode.
Also note that you will need to make LdapException a sub-class of
j.u.c.ExecutionException for it to be thrown by Future.get (or make it a
runtime exception but I think that this is a bad idea). This is a bit
annoying, but in practice not a big deal (it just looks surprising
seeing java.util.concurrent in the class hierarchy for a result exception).
Another API problem I ran into was what to do with the "checked"
InterruptedException which can be thrown from blocking operations such
as Future.get. I could have chosen to catch it and rethrow it as a
cancelled result exception (or a new exception like
InterruptedErrorResultException). This would avoid having to catch/throw
it every time, as this example illustrates:
Connection connection = ...;
Entry entry = ...;
try
{
connection.add(entry);
}
catch (ErrorResultException e)
{
// Handle operation failure.
}
catch (InterruptedException e)
{
// Grrr... Handle thread interrupt
// This would not be needed if I caught and re-threw
// the exception as a sub-type of ErrorResultException.
}
I played it safe and kept it separate since InterruptedException has a
very specific contract, so hiding it inside an ErrorResultException (or
LdapException in your case) might cause it to get overlooked. Brian
Goetz talks about it here:
http://www.ibm.com/developerworks/java/library/j-jtp05236.html
Having said that, every time I look at our APIs and see the "throws
InterruptedException" I have to restrain myself as I am still very
tempted to catch and rethrow as a sub-type of ErrorResultException! :-)
Matt
On 19/07/10 16:49, Emmanuel Lecharny wrote:
> Hi,
>
> long time, no see :)
>
> I have a proposal about the way we handle the operation results.
> Currently, every operation is returning a Response, and if we want to
> know if the operation has been successful, we have to check the
> LdapResult field. This is not very convenient.
>
> Assuming that when we have issued a synchronous operation, we are
> waiting until we get a response, why can't we have those operations
> throwing a LdapException ?
>
> For async operations, we can also make that the XXXFuture.get() method
> throw the same exception.
>
> thoughts ?
>