I think this is acceptable too:
catch (AException e)
{
log.warn("Got an AException whilst trying to do ...", e);
throw new BException(e);
}
The idea being that logging something as an error counts as handling it, and
means that it is something you really ought to be taking note of. Logging as
a warn doesn't count as handling it, its just a stronger form of info.
Rupert
On 10/04/07, Rupert Smith <[EMAIL PROTECTED]> wrote:
Ok, but lets draw a distinction between the exception and the handling of
it.
The exception is just a container for information relating to the error
condition that gave rise to its creation. Its role should be purely passive.
It should probably be like a java bean, with getters and setters to
attach/read various properties of the error.
The handling of it, is what goes in a catch block (or maybe some piece of
re-usable handling code you managed to factor out). Logging is a form of
handling an exception, (which is why I object to it being in the exception
itself).
You only want to handle an exception once, ideally. I wouldn't count
re-casting or wrapping an exception as handling it, the purpose of that is
just to reshape it to fit declared exception types in interfaces. This is
why I wouldn't log it as an error and then rethrow it, because logging it
counts as handling, and rethrowing implies that somewhere higher up is doing
the handling.
There should always be a top-level error handler, in the main method, or
at the top of each worker thread, or some similarly top-level location as
appropriate. This just provides default handling for all exceptions that
make it through and guarantees that all exceptions are handled.
Rupert
On 10/04/07, Marnie McCormack <[EMAIL PROTECTED]> wrote:
>
> Not sure we can make any useful assumptions about what the Qpid error
> logging/exception handling will definitely do higher up though :-)
>
> M
>
>
> On 4/10/07, Rupert Smith <[EMAIL PROTECTED]> wrote:
> >
> > Yes, that would do too.
> >
> > On 10/04/07, Martin Ritchie < [EMAIL PROTECTED]> wrote:
> > >
> > > I would have suggested a log of
> > >
> > > log.debug/trace("Recasting TypeAE to TypeBE");
> > >
> > > So the trace can be shown though the log.
> > >
> > > On 10/04/07, Rupert Smith <[EMAIL PROTECTED]> wrote:
> > > > This error logging doesn't entirely convince me either:
> > > >
> > > > catch (TypeAException e)
> > > > {
> > > > log.error("Went wrong!", e);
> > > > throw new TypeBException(e);
> > > > }
> > > >
> > > > I think I would just do:
> > > >
> > > > catch (TypeAException e)
> > > > {
> > > > throw new TypeBException(e);
> > > > }
> > > >
> > > > The rethrow here is simply to re-cast the original exception as a
> > > different
> > > > type. Presumably it will be logged as an error again somewhere
> higher
> > > up.
> > > >
> > > > Rupert
> > > >
> > > > On 09/04/07, Rupert Smith < [EMAIL PROTECTED]> wrote:
> > > > >
> > > > > One problem I've often found with exceptions, is the hassle of
> > writing
> > > so
> > > > > many constructors. One for just a message, one for message +
> wrapped
> > > > > exception, one for message + error code, and every permutation
> > > thereof. A
> > > > > simple scheme I've used previously to avoid this is simply to
> allow
> > > > > parameters in exception constructor to be null, if they are not
> to
> > be
> > > set,
> > > > > and just always use a single constructor. For example:
> > > > >
> > > > > /**
> > > > > * Root of the application exception hierarchy.
> > > > > */
> > > > > public class MyException extends Exception
> > > > > {
> > > > > /**
> > > > > * @param message May be null if not to be set.
> > > > > * @param code May be null if not to be set.
> > > > > * @param cause May be null if not to be set.
> > > > > */
> > > > > public MyException(String message, Integer code, Throwable
> cause)
> > > > > {
> > > > > super(message == null ? "" : message, cause);
> > > > > this._errorCode = code == null ? 0 : code.intValue();
> > > > > ...
> > > > > }
> > > > > }
> > > > >
> > > > >
> > > > > ...
> > > > > throw new MyException("Went wrong.", null, null);
> > > > >
> > > > > Some people might object to the nulls, but it does take the pain
> out
> > > of
> > > > > writing exception classes.
> > > > >
> > > > > Rupert
> > > > >
> > > > > On 09/04/07, Rupert Smith <[EMAIL PROTECTED]> wrote:
> > > > > >
> > > > > > Although, I notice that there is a JMSAMQException
> specifically
> > for
> > > the
> > > > > > case where an AMQException is to be rethrown as a
> JMSException.
> > > > > >
> > > > > > On 09/04/07, Rupert Smith <[EMAIL PROTECTED]>
> wrote:
> > > > > > >
> > > > > > > Yes, there's quite a lot of it in there. I'm going to leave
> some
> > > of it
> > > > > > > well alone for the moment, but fix some things that don't
> really
> > > alter the
> > > > > > > semantics of the code:
> > > > > > >
> > > > > > > Here's one. Don't do this:
> > > > > > >
> > > > > > > catch (SomeException e)
> > > > > > > {
> > > > > > > throw new MyException("Something went wrong.");
> > > > > > > }
> > > > > > >
> > > > > > > Do this instead:
> > > > > > >
> > > > > > > catch (SomeException e)
> > > > > > > {
> > > > > > > throw new MyException("Something went wrong.", e);
> > > > > > > }
> > > > > > >
> > > > > > > of for JMSException which doesn't accept wrapped exceptions
> > > through
> > > > > > > its constructors, have to do something like:
> > > > > > >
> > > > > > > catch (SomeException e)
> > > > > > > {
> > > > > > > JMSException jmse = new JMSException("Something went
> wrong.");
> > > > > > > jmse.setLinkedException(e);
> > > > > > > throw jmse;
> > > > > > > }
> > > > > > >
> > > > > > > This isn't majorly wrong, just annoying to lose half the
> > exception
> > > > > > > stack trace, when tracking down bugs from log files.
> > > > > > >
> > > > > > > Rupert
> > > > > > >
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> > >
> > > --
> > > Martin Ritchie
> > >
> >
>
