Re: [jgit-dev] Exceptions in jgit

["Shawn O. Pearce" <spearce@xxxxxxxxxxx>]

"Zivkov, Sasa" <sasa.zivkov@xxxxxxx> wrote:
> I am trying to figure out what is the approach to usage of exceptions in jgit.
> Looking at the org.eclipse.jgit.errors package, where the jgit exceptions are defined,
> I see mainly 3 cases:
> 1. An exception is subtype of java.io.IOException
> 2. An exception is subtype of java.lang.Exception
> 3. An exception is subtype of java.lang.RuntimeException
> 
> However, I don't see the rule. If I would introduce a new exception I wouldn't
> know which approach to use.

Heh.  We don't have a good exception system.  Let me explain how
we got where we are, its about the best rule we have...

Back when I started JGit I didn't feel like handling IOException
when reading from a file, so I declared it as a thrown exception
to the caller.

Eventually I realized I should handle certain common problems the
same way.  E.g.  MissingObjectException.  So that rather than throw
new IOException("missing blob "+id) I could instead do throw new
MissingObjectException(id) and have the message be computed the
same way every time.

Only my interfaces were already throwing IOException.  Adding the
new type MissingObjectException as a checked exception to all of
the throws clauses got messy fast.  So it subclassed IOException.

Newer exceptions like InvalidPatternException extend from Exception
so they must be declared, and so they don't get sucked up inside
of a super-generic catch IOException block.

At some point I realized extending from IOException is a mess,
so the transport stuff all extends from TransportException... but
that itself extends from IOException, because I'm a moron.


Then you have some oddballs like InvalidObjectIdException is
extending IllegalArgumentException.  Because the method that throws
IOIE used to throw IAE.  Now it throws a more specific subclass of
IAE, but is still throwing IAE.


Finally, StopWalkException is thrown and caught only deep down within
the RevWalk code path.  It extends RuntimeException so I didn't
need to go mark everyone who could possibly throw it.  Since this
is mostly an internal exception, that could probably be changed to
a checked Exception type instead with relatively little pain.

RevWalkException extends RuntimeException because its thrown out
of the hasNext() or next() methods of an Iterator<RevCommit> when
being invoked by RevWalk and a checked IOException was thrown by
some of the lazy disk access code paths.


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.

-- 
Shawn.