You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@directory.apache.org by Emmanuel Lécharny <el...@gmail.com> on 2013/06/25 14:03:03 UTC

Error handling in the server

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 


Re: Error handling in the server

Posted by Kiran Ayyagari <ka...@apache.org>.
On Tue, Jun 25, 2013 at 5:33 PM, Emmanuel Lécharny <el...@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 ?
>
> +1

> --
> Regards,
> Cordialement,
> Emmanuel Lécharny
> www.iktek.com
>
>


-- 
Kiran Ayyagari
http://keydap.com

Re: Error handling in the server

Posted by Pierre-Arnaud Marcelot <pa...@marcelot.net>.
Makes totally sense to me.

Regards,
Pierre-Arnaud

On 25 juin 2013, at 14:03, Emmanuel Lécharny <el...@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 
>