|Re: [jgit-dev] Exceptions in jgit|
"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 220.127.116.11. I've basically written it off as something I just can't do. -- Shawn.
Back to the top