Mailing-List: contact api-help@directory.apache.org; run by ezmlm
Precedence: bulk
Reply-To: api@directory.apache.org
Received-SPF: pass (nike.apache.org: local policy)
Message-ID: <4C448519.5080807@oracle.com>
Date: Mon, 19 Jul 2010 19:02:17 +0200
From: Matthew Swift <matthew.swift@oracle.com>
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US;
 rv:1.9.2.4) Gecko/20100608 Lightning/1.0b2
 OracleBeehiveExtension/1.0.0.0pre11-OracleInternal Thunderbird/3.1
MIME-Version: 1.0
To: api@directory.apache.org
Subject: Re: About operation result
References: <4C4465F0.7060804@gmail.com>
In-Reply-To: <4C4465F0.7060804@gmail.com>
Content-Type: multipart/alternative;
 boundary="------------090105030906070300030106"

--------------090105030906070300030106
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

  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 ?
>

--------------090105030906070300030106--