You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Rupert Smith <ru...@googlemail.com> on 2007/04/09 15:40:22 UTC
More Bad Exception Handling.
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
Re: More Bad Exception Handling.
Posted by Rupert Smith <ru...@googlemail.com>.
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 <ru...@googlemail.com> 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 <ma...@googlemail.com> 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 <ru...@googlemail.com> wrote:
> > >
> > > Yes, that would do too.
> > >
> > > On 10/04/07, Martin Ritchie < ritchiem@apache.org> 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 <ru...@googlemail.com> 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 < rupertlssmith@googlemail.com> 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 <ru...@googlemail.com> 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 <ru...@googlemail.com>
> > 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
> > > >
> > >
> >
>
>
Re: More Bad Exception Handling.
Posted by Rupert Smith <ru...@googlemail.com>.
Don't worry Marnie, I'm not planning on making any changes at the moment.
The above discussion has been helpfull though, in getting a more solid idea
of how it will look. Eventually, when there's time...
Rupert
On 12/04/07, Marnie McCormack <ma...@googlemail.com> wrote:
>
> I agree with the vast majority of the discussion (consensus anyway).
>
> We have loads of stuff in JIRAs etc which imho come before rewriting all
> our
> exception handling :-) In light of this, wanted to re-iterate that making
> wholesale changes would be lovely - but in the absence of that, please
> take
> care when making assumptions about how thrown exceptions will be handled
> higher up.
>
> I don't much care where we log exceptions - so long as we do. Many
> (most/all
> ??) user applications using Qpid will use log monitoring to health check
> their deployments.
>
> Logging on construction has been used in past projects as a fool proof way
> to make sure that exceptions are always logged appropriately. I'd far
> rather
> take the chance that we log (handle) twice in a slightly impure way than
> (as
> we perhaps too often do), catch and do nothing or keep throwing upwards
> with
> the assumption that some calling method cares enough to log it !
>
> Be different if we were having this discussion at project inception. Since
> we're not, let's be realistic about how much time this really justifies !
>
> Bfn,
> Marnie
>
>
>
> On 4/11/07, Robert Godfrey <ro...@gmail.com> wrote:
> >
> > Yes - In general I'm of the opinion that if you get any exception
> bubbling
> > up to the top level handler then you should probably quite... but that
> may
> > be a bit severe for the moment.
> >
> > By top level, obviously what we mean is some sort of uncaught exception
> > handler for the thread group which all threads are running in.
> >
> > -- Rob
> >
> > On 11/04/07, Rupert Smith <ru...@googlemail.com> wrote:
> > >
> > > As a top-level handler? To avoid littering it throughout the code?
> Then
> > we
> > > can follow the general principal, that when something bad happens that
> > you
> > > cannot deal with, raise it and let it fall through all the way to this
> > > handler.
> > >
> > > On 11/04/07, Robert Godfrey <ro...@gmail.com> wrote:
> > > >
> > > > Generally I think
> > > >
> > > > catch(Error e)
> > > > {
> > > > try
> > > > {
> > > > // try to do some log here perhaps
> > > > }
> > > > finally
> > > > {
> > > > System.exit(1);
> > > > }
> > > > }
> > > >
> > > > Error is bad :-)
> > > >
> > > > On 11/04/07, Martin Ritchie <ri...@apache.org> wrote:
> > > > >
> > > > > For OOM I would agree that recover mode == System.exit :)
> > > > >
> > > > > On 11/04/07, Rupert Smith <ru...@googlemail.com> wrote:
> > > > > > I'm in definite agreement with this point of view.
> > > > > >
> > > > > > As I go through the errors I'm taking note of suspiciously
> > unhandled
> > > > > > conditions, and should get around to turning them into JIRA's at
> > > some
> > > > > point.
> > > > > >
> > > > > > Rupert
> > > > > >
> > > > > > On 11/04/07, Robert Godfrey <ro...@gmail.com> wrote:
> > > > > > >
> > > > > > > > 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.
> > > > > > >
> > > > > > >
> > > > > > > I shall repeat my view that an OOM error should bring
> > the broker
> > > to
> > > > > and
> > > > > > > *abrupt* halt (rather than a grinding one) since there is
> > > *nothing*
> > > > > you
> > > > > > > can
> > > > > > > reliably do once you get to this point. The trick is to try
> to
> > > > ensure
> > > > > > > that
> > > > > > > you never hit this point.
> > > > > > >
> > > > > > > However the best thing to do with unrecoverable errors is to
> > exit
> > > > > quickly
> > > > > > > and as cleanly as possible.
> > > > > > >
> > > > > > > -- Rob
> > > > > > >
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Martin Ritchie
> > > > >
> > > >
> > >
> >
>
Re: More Bad Exception Handling.
Posted by Marnie McCormack <ma...@googlemail.com>.
I agree with the vast majority of the discussion (consensus anyway).
We have loads of stuff in JIRAs etc which imho come before rewriting all our
exception handling :-) In light of this, wanted to re-iterate that making
wholesale changes would be lovely - but in the absence of that, please take
care when making assumptions about how thrown exceptions will be handled
higher up.
I don't much care where we log exceptions - so long as we do. Many (most/all
??) user applications using Qpid will use log monitoring to health check
their deployments.
Logging on construction has been used in past projects as a fool proof way
to make sure that exceptions are always logged appropriately. I'd far rather
take the chance that we log (handle) twice in a slightly impure way than (as
we perhaps too often do), catch and do nothing or keep throwing upwards with
the assumption that some calling method cares enough to log it !
Be different if we were having this discussion at project inception. Since
we're not, let's be realistic about how much time this really justifies !
Bfn,
Marnie
On 4/11/07, Robert Godfrey <ro...@gmail.com> wrote:
>
> Yes - In general I'm of the opinion that if you get any exception bubbling
> up to the top level handler then you should probably quite... but that may
> be a bit severe for the moment.
>
> By top level, obviously what we mean is some sort of uncaught exception
> handler for the thread group which all threads are running in.
>
> -- Rob
>
> On 11/04/07, Rupert Smith <ru...@googlemail.com> wrote:
> >
> > As a top-level handler? To avoid littering it throughout the code? Then
> we
> > can follow the general principal, that when something bad happens that
> you
> > cannot deal with, raise it and let it fall through all the way to this
> > handler.
> >
> > On 11/04/07, Robert Godfrey <ro...@gmail.com> wrote:
> > >
> > > Generally I think
> > >
> > > catch(Error e)
> > > {
> > > try
> > > {
> > > // try to do some log here perhaps
> > > }
> > > finally
> > > {
> > > System.exit(1);
> > > }
> > > }
> > >
> > > Error is bad :-)
> > >
> > > On 11/04/07, Martin Ritchie <ri...@apache.org> wrote:
> > > >
> > > > For OOM I would agree that recover mode == System.exit :)
> > > >
> > > > On 11/04/07, Rupert Smith <ru...@googlemail.com> wrote:
> > > > > I'm in definite agreement with this point of view.
> > > > >
> > > > > As I go through the errors I'm taking note of suspiciously
> unhandled
> > > > > conditions, and should get around to turning them into JIRA's at
> > some
> > > > point.
> > > > >
> > > > > Rupert
> > > > >
> > > > > On 11/04/07, Robert Godfrey <ro...@gmail.com> wrote:
> > > > > >
> > > > > > > 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.
> > > > > >
> > > > > >
> > > > > > I shall repeat my view that an OOM error should bring
> the broker
> > to
> > > > and
> > > > > > *abrupt* halt (rather than a grinding one) since there is
> > *nothing*
> > > > you
> > > > > > can
> > > > > > reliably do once you get to this point. The trick is to try to
> > > ensure
> > > > > > that
> > > > > > you never hit this point.
> > > > > >
> > > > > > However the best thing to do with unrecoverable errors is to
> exit
> > > > quickly
> > > > > > and as cleanly as possible.
> > > > > >
> > > > > > -- Rob
> > > > > >
> > > > >
> > > >
> > > >
> > > > --
> > > > Martin Ritchie
> > > >
> > >
> >
>
Re: More Bad Exception Handling.
Posted by Robert Godfrey <ro...@gmail.com>.
Yes - In general I'm of the opinion that if you get any exception bubbling
up to the top level handler then you should probably quite... but that may
be a bit severe for the moment.
By top level, obviously what we mean is some sort of uncaught exception
handler for the thread group which all threads are running in.
-- Rob
On 11/04/07, Rupert Smith <ru...@googlemail.com> wrote:
>
> As a top-level handler? To avoid littering it throughout the code? Then we
> can follow the general principal, that when something bad happens that you
> cannot deal with, raise it and let it fall through all the way to this
> handler.
>
> On 11/04/07, Robert Godfrey <ro...@gmail.com> wrote:
> >
> > Generally I think
> >
> > catch(Error e)
> > {
> > try
> > {
> > // try to do some log here perhaps
> > }
> > finally
> > {
> > System.exit(1);
> > }
> > }
> >
> > Error is bad :-)
> >
> > On 11/04/07, Martin Ritchie <ri...@apache.org> wrote:
> > >
> > > For OOM I would agree that recover mode == System.exit :)
> > >
> > > On 11/04/07, Rupert Smith <ru...@googlemail.com> wrote:
> > > > I'm in definite agreement with this point of view.
> > > >
> > > > As I go through the errors I'm taking note of suspiciously unhandled
> > > > conditions, and should get around to turning them into JIRA's at
> some
> > > point.
> > > >
> > > > Rupert
> > > >
> > > > On 11/04/07, Robert Godfrey <ro...@gmail.com> wrote:
> > > > >
> > > > > > 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.
> > > > >
> > > > >
> > > > > I shall repeat my view that an OOM error should bring the broker
> to
> > > and
> > > > > *abrupt* halt (rather than a grinding one) since there is
> *nothing*
> > > you
> > > > > can
> > > > > reliably do once you get to this point. The trick is to try to
> > ensure
> > > > > that
> > > > > you never hit this point.
> > > > >
> > > > > However the best thing to do with unrecoverable errors is to exit
> > > quickly
> > > > > and as cleanly as possible.
> > > > >
> > > > > -- Rob
> > > > >
> > > >
> > >
> > >
> > > --
> > > Martin Ritchie
> > >
> >
>
Re: More Bad Exception Handling.
Posted by Rupert Smith <ru...@googlemail.com>.
As a top-level handler? To avoid littering it throughout the code? Then we
can follow the general principal, that when something bad happens that you
cannot deal with, raise it and let it fall through all the way to this
handler.
On 11/04/07, Robert Godfrey <ro...@gmail.com> wrote:
>
> Generally I think
>
> catch(Error e)
> {
> try
> {
> // try to do some log here perhaps
> }
> finally
> {
> System.exit(1);
> }
> }
>
> Error is bad :-)
>
> On 11/04/07, Martin Ritchie <ri...@apache.org> wrote:
> >
> > For OOM I would agree that recover mode == System.exit :)
> >
> > On 11/04/07, Rupert Smith <ru...@googlemail.com> wrote:
> > > I'm in definite agreement with this point of view.
> > >
> > > As I go through the errors I'm taking note of suspiciously unhandled
> > > conditions, and should get around to turning them into JIRA's at some
> > point.
> > >
> > > Rupert
> > >
> > > On 11/04/07, Robert Godfrey <ro...@gmail.com> wrote:
> > > >
> > > > > 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.
> > > >
> > > >
> > > > I shall repeat my view that an OOM error should bring the broker to
> > and
> > > > *abrupt* halt (rather than a grinding one) since there is *nothing*
> > you
> > > > can
> > > > reliably do once you get to this point. The trick is to try to
> ensure
> > > > that
> > > > you never hit this point.
> > > >
> > > > However the best thing to do with unrecoverable errors is to exit
> > quickly
> > > > and as cleanly as possible.
> > > >
> > > > -- Rob
> > > >
> > >
> >
> >
> > --
> > Martin Ritchie
> >
>
Re: More Bad Exception Handling.
Posted by Robert Godfrey <ro...@gmail.com>.
Generally I think
catch(Error e)
{
try
{
// try to do some log here perhaps
}
finally
{
System.exit(1);
}
}
Error is bad :-)
On 11/04/07, Martin Ritchie <ri...@apache.org> wrote:
>
> For OOM I would agree that recover mode == System.exit :)
>
> On 11/04/07, Rupert Smith <ru...@googlemail.com> wrote:
> > I'm in definite agreement with this point of view.
> >
> > As I go through the errors I'm taking note of suspiciously unhandled
> > conditions, and should get around to turning them into JIRA's at some
> point.
> >
> > Rupert
> >
> > On 11/04/07, Robert Godfrey <ro...@gmail.com> wrote:
> > >
> > > > 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.
> > >
> > >
> > > I shall repeat my view that an OOM error should bring the broker to
> and
> > > *abrupt* halt (rather than a grinding one) since there is *nothing*
> you
> > > can
> > > reliably do once you get to this point. The trick is to try to ensure
> > > that
> > > you never hit this point.
> > >
> > > However the best thing to do with unrecoverable errors is to exit
> quickly
> > > and as cleanly as possible.
> > >
> > > -- Rob
> > >
> >
>
>
> --
> Martin Ritchie
>
Re: More Bad Exception Handling.
Posted by Martin Ritchie <ri...@apache.org>.
For OOM I would agree that recover mode == System.exit :)
On 11/04/07, Rupert Smith <ru...@googlemail.com> wrote:
> I'm in definite agreement with this point of view.
>
> As I go through the errors I'm taking note of suspiciously unhandled
> conditions, and should get around to turning them into JIRA's at some point.
>
> Rupert
>
> On 11/04/07, Robert Godfrey <ro...@gmail.com> wrote:
> >
> > > 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.
> >
> >
> > I shall repeat my view that an OOM error should bring the broker to and
> > *abrupt* halt (rather than a grinding one) since there is *nothing* you
> > can
> > reliably do once you get to this point. The trick is to try to ensure
> > that
> > you never hit this point.
> >
> > However the best thing to do with unrecoverable errors is to exit quickly
> > and as cleanly as possible.
> >
> > -- Rob
> >
>
--
Martin Ritchie
Re: More Bad Exception Handling.
Posted by Rupert Smith <ru...@googlemail.com>.
I'm in definite agreement with this point of view.
As I go through the errors I'm taking note of suspiciously unhandled
conditions, and should get around to turning them into JIRA's at some point.
Rupert
On 11/04/07, Robert Godfrey <ro...@gmail.com> wrote:
>
> > 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.
>
>
> I shall repeat my view that an OOM error should bring the broker to and
> *abrupt* halt (rather than a grinding one) since there is *nothing* you
> can
> reliably do once you get to this point. The trick is to try to ensure
> that
> you never hit this point.
>
> However the best thing to do with unrecoverable errors is to exit quickly
> and as cleanly as possible.
>
> -- Rob
>
Re: More Bad Exception Handling.
Posted by Robert Godfrey <ro...@gmail.com>.
> 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.
I shall repeat my view that an OOM error should bring the broker to and
*abrupt* halt (rather than a grinding one) since there is *nothing* you can
reliably do once you get to this point. The trick is to try to ensure that
you never hit this point.
However the best thing to do with unrecoverable errors is to exit quickly
and as cleanly as possible.
-- Rob
Re: More Bad Exception Handling.
Posted by Martin Ritchie <ri...@apache.org>.
On 10/04/07, Rupert Smith <ru...@googlemail.com> 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 <ma...@googlemail.com> 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 <ru...@googlemail.com> wrote:
> > >
> > > Yes, that would do too.
> > >
> > > On 10/04/07, Martin Ritchie <ri...@apache.org> 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 <ru...@googlemail.com> 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 <ru...@googlemail.com> 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 <ru...@googlemail.com> 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 <ru...@googlemail.com> 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
Re: More Bad Exception Handling.
Posted by Rupert Smith <ru...@googlemail.com>.
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 <ma...@googlemail.com> 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 <ru...@googlemail.com> wrote:
> >
> > Yes, that would do too.
> >
> > On 10/04/07, Martin Ritchie <ri...@apache.org> 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 <ru...@googlemail.com> 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 <ru...@googlemail.com> 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 <ru...@googlemail.com> 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 <ru...@googlemail.com> 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
> > >
> >
>
Re: More Bad Exception Handling.
Posted by Marnie McCormack <ma...@googlemail.com>.
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 <ru...@googlemail.com> wrote:
>
> Yes, that would do too.
>
> On 10/04/07, Martin Ritchie <ri...@apache.org> 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 <ru...@googlemail.com> 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 <ru...@googlemail.com> 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 <ru...@googlemail.com> 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 <ru...@googlemail.com> 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
> >
>
Re: More Bad Exception Handling.
Posted by Rupert Smith <ru...@googlemail.com>.
Yes, that would do too.
On 10/04/07, Martin Ritchie <ri...@apache.org> 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 <ru...@googlemail.com> 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 <ru...@googlemail.com> 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 <ru...@googlemail.com> 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 <ru...@googlemail.com> 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
>
Re: More Bad Exception Handling.
Posted by Martin Ritchie <ri...@apache.org>.
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 <ru...@googlemail.com> 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 <ru...@googlemail.com> 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 <ru...@googlemail.com> 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 <ru...@googlemail.com> 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
Re: More Bad Exception Handling.
Posted by Rupert Smith <ru...@googlemail.com>.
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 <ru...@googlemail.com> 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 <ru...@googlemail.com> 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 <ru...@googlemail.com> 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
> > >
> >
> >
>
Re: More Bad Exception Handling.
Posted by Rupert Smith <ru...@googlemail.com>.
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 <ru...@googlemail.com> 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 <ru...@googlemail.com> 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
> >
>
>
Re: More Bad Exception Handling.
Posted by Rupert Smith <ru...@googlemail.com>.
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 <ru...@googlemail.com> 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
>
Re: More Bad Exception Handling.
Posted by Martin Ritchie <ri...@apache.org>.
On 09/04/07, Rupert Smith <ru...@googlemail.com> 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
>
We did get fed up of the diong the .setLinkedException on JMSE so we
created AMQJMSException (or is it JMSAMQException) that does the
correct wrapping :)
--
Martin Ritchie