directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Pierre-Arnaud Marcelot ...@marcelot.net>
Subject Re: Error handling in the server
Date Tue, 25 Jun 2013 13:10:46 GMT
Makes totally sense to me.

Regards,
Pierre-Arnaud

On 25 juin 2013, at 14:03, Emmanuel L├ęcharny <elecharny@gmail.com> wrote:

> Hi guys,
> 
> atm, when we get an exception in the server, it's handled at the handler
> level. For instance, the ModifyRequestHandler code is like this :
> 
>    public void handle( LdapSession session, ModifyRequest req )
>    {
>        LdapResult result = req.getResultResponse().getLdapResult();
> 
>        try
>        {
>            ...
>            result.setResultCode( ResultCodeEnum.SUCCESS );
> 
>            session.getIoSession().write( req.getResultResponse() );
>        }
>        catch ( Exception e )
>        {
>            handleException( session, req, e );
>        }
> 
> As we can see, either we have a success, or an exception. In the second
> case, we call the handleException which does :
> 
>    public void handleException( LdapSession session,
> ResultResponseRequest<?> req, Exception e )
>    {
>        LdapResult result = req.getResultResponse().getLdapResult();
> 
>        ResultCodeEnum code;
> 
>        if ( e instanceof LdapOperationException )
>        {
>            code = ( ( LdapOperationException ) e ).getResultCode();
>        }
>        else
>        {
>            code = ResultCodeEnum.getBestEstimate( e, req.getType() );
>        }
> 
>        result.setResultCode( code );
> 
>        String msg = code.toString() + ": failed for " + req + ": " +
> e.getLocalizedMessage();
> 
>        if ( LOG.isDebugEnabled() )
>        {
>            LOG.debug( msg, e );
> 
>            msg += ":\n" + ExceptionUtils.getStackTrace( e );
>        }
> 
>        result.setDiagnosticMessage( msg );
> 
>        if ( e instanceof LdapOperationException )
>        {
>           ...
>        }
> 
>        session.getIoSession().write( req.getResultResponse() );
>    }
> 
> Here, we never return the real cause of the exception if it's not a LDAP
> standard error, we only dump the stackTrace if we have set the LOG level
> to debug.
> 
> There are two things I'd like to change :
> 
> - First, we should always log at the ERROR level, not debug
> - Second, when the error is OTHER, it's really likely we have something
> wrong in the server that needs to be detected. I would like to send back
> a StackTrace in this case.
> 
> The reason is that it would help us to eliminate errors like NPE that
> simply get swallowed, which makes it extremelly difficult to understand
> what's going on when a user reports an error.
> 
> 
> wdyt ?
> 
> -- 
> Regards,
> Cordialement,
> Emmanuel L├ęcharny
> www.iktek.com 
> 


Mime
View raw message