Why you probably don't want to catch java.lang.Throwable

0 views
Skip to first unread message

rnicoll

unread,
Jun 25, 2008, 5:19:40 AM6/25/08
to jsystemtrader-dev
Just sat down and started looking over the code (I figure I'm better
off hacking at your code than continuing with the mess I've made of my
own attempts)...

First thing I notice, is you're catching Throwable. For example,
com.jsystemtrader.platform.trader.Trader, line 149:

} catch (Throwable t) {
// Do not allow exceptions come back to the socket -- it will
cause disconnects
eventReport.report(t);
}

Which looks perfectly sensible on first glance; we don't want errors
in the strategy getting back to TWS. However... this means, for
example, you're catching OutOfMemoryError, which means it never gets
up to the top of the VM to have it auto-trigger the garbage collector.
You're also catching InternalError, "Thrown to indicate some
unexpected internal error has occurred in the Java Virtual Machine. "
- I don't think at that point the code should be quietly reporting
then dropping, I think it should be shutting down ASAP. Possibly
thinking about closing out all the trades on the way through, but
mostly shutting down.

Anyone object if I start going through these, and modifying them to
catch Exception instead? I don't meant with a search & replace, I
promise to actually check it's valid to make the changes before I do
them!

Eugene Kononov

unread,
Jun 25, 2008, 6:49:43 AM6/25/08
to jsystemt...@googlegroups.com
I agree that catching Throwable is generally not a good programming practice. Feel free to replace them with more specific exceptions.

J Ross Nicoll

unread,
Jun 25, 2008, 8:59:27 AM6/25/08
to jsystemt...@googlegroups.com
This expands on the exceptions thrown in
com.jsystemtrader.platform.util.Browser to make it clearer what's
happening, and wraps them all in a BrowserUnavailableException or
subclass thereof. The actions that call Browser in MainController then
only catch BrowserUnavailableException instead of Throwable.

3 down, 22 to go.

browser_throwable.diff

Eugene Kononov

unread,
Jun 25, 2008, 9:58:54 AM6/25/08
to jsystemt...@googlegroups.com
Thanks, JRN. I just wanted to mention that Florent is managing JST more actively than I do, and he loves patches from contributors, so in all likelihood, your patch will make it to the source tree soon.

We also need more developers in the JBookTrader project (which is also managed by Florent and myself), and you are certainly welcomed to join:
http://code.google.com/p/jbooktrader/

Florent Guiliani

unread,
Jun 25, 2008, 10:16:41 AM6/25/08
to jsystemt...@googlegroups.com
Thank you very much for your patch. I'm pretty busy thoses days but I soon I
will get more free time, I'll return to the JST & JBT development and add your
patch into an official release.

J Ross Nicoll

unread,
Jun 25, 2008, 10:30:15 AM6/25/08
to jsystemt...@googlegroups.com
JBookTrader , from a quick look, is for strategies based on market depth rather than data bars? I'm focusing on minute-bar based strategies at the moment, but I'll have a look when I have more free time (err... how's 2010? :) )

J Ross Nicoll

unread,
Jun 25, 2008, 10:31:00 AM6/25/08
to jsystemt...@googlegroups.com
No hurry, although if you leave it too long you might find yourself a
little swamped by patches (I'm a big believer in producing lots of
small patches so each step of progression is clearer).

Eugene Kononov

unread,
Jun 25, 2008, 11:07:28 AM6/25/08
to jsystemt...@googlegroups.com

No hurry, although if you leave it too long you might find yourself a
little swamped by patches (I'm a big believer in producing lots of
small patches so each step of progression is clearer).


That's fine.
 
>JBookTrader , from a quick look, is for strategies based on market depth rather than data bars?

Yes, JBT is for market depth trading.
Reply all
Reply to author
Forward
0 new messages