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.
And there is the current problem. we don't have top-level error
handlers in place in the broker. Hence things like OutOfMemory Errors
cause the broker to grind to a halt rather than entering any recovery
mode.
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
> > >
> >
>
--
Martin Ritchie
