Re: [jgit-dev] Exceptions in jgit

[Alex Blewitt <alex.blewitt@xxxxxxxxx>]

On 11 Mar 2010, at 15:38, Shawn O. Pearce wrote:

> "Zivkov, Sasa" <sasa.zivkov@xxxxxxx> wrote:
>>> I'd like to move to more specific checked exceptions, and stop
>>> throwing back any old IOException to the application caller.
>>> But this is a fairly major effort, and I'm just struggling to
>>> get the sideband protcol change implemented to match Git 1.7.0.2.
>>> I've basically written it off as something I just can't do.
>> 
>> Why checked exceptions?
>> I don't want to start yet another discussion on checked vs non-checked
>> exceptions in general since it often goes into discussing
>> personal preferences.
> 
> Quite true.  This is similar to that emacs/vi debate.  Nah, its
> more like the debate about whether or not there should be a blank
> line between field declarations in a Java class, because everyone
> on the project has to live with one guy's opinion.  :-)

Caution: contains another guy's opinion :-)

Personally, I think the project should adopt one style and stick with it. I think it's more dangerous to have some methods having one style and then others silently throwing RuntimeExceptions where they weren't expected. 

I've also seen a bunch of Eclipse code wrap up (at the top layer) code that just results in a try/catch Exception, such that it gets logged to the log itself. The net effect to end users is that they click on a button, and nothing happens. They click on it again, and nothing happens. All that happens is they get frustrated and then there's a pileup on the log. This is mainly when an unhandled RuntimeException goes up the stack; and that could be anywhere. It's not Eclipse's fault (and in some cases, not the plugin's either) but the end user doesn't see it.

It's also very easy to convert a checked exception into a runtime exception if needed. The reverse is almost impossible, because you never know where they are.  So if someone using the JGit library really, really wants RuntimeExceptions they can write a wrapper class that fields it on. Heck, you could probably even auto-generate those ...

So, if we're writing a generic JGit library for any IDE which needs to report errors in a customisable way (i.e. not just System.out.println or try/catch/ginore) then you *have* to report that in a way that library callers know how to use. Anything buried in a RuntimeException has no real hope of translating into anything other than a message in a log-file or a somewhat less than helpful generic error box.

My thoughts are that libraries should declare checked exceptions, and make it part of the contract. I don't necessarily buy that you have to wrap everything in a JGitException - if you've got IO on the path, then having JGitException and IOException are fine. (We *really* don't want to be wrapping IOExceptions in RuntimeExceptions; that's basically the try/catch/ignore pattern.)

Speaking of exceptions, I think we should be creating specific exception subclasses rather than a generic exception with a message.  That makes it easier to build specific customised error handling (whether that's message or retry operations) rather than using a switch on getMessage()). It should also be possible to embed specific data to the problem (like the objectid) which can be presented in an appropriate manner.

Alex