Hi,
Early JGit code seems to be a little confused about how to handle I/O errors. For example:
public Map<String, Ref> getAllRefs() {
try {
return getRefDatabase().getRefs(RefDatabase.ALL);
} catch (IOException e) {
return new HashMap<>();
}
}
This is dangerous and flaky. I'd like to deprecate any interfaces that do this kind of thing and vow to never do it again.
That opens a question of what methods like getAllRefs should throw. Our current answer has been to throw IOException and make all *other* ways the function can fail also inherit from IOException:
https://imgur.com/a/k8yYPaP
This makes IOException behave like a cross between Exception and RuntimeException. Like RuntimeException, callers can expect a large part of JGit's API to throw it without declaring which subclasses they throw. Like Exception, the only way to handle it is to log and bail out, since the exception type does not indicate what actually happened in an actionable way. Methods can throw new subclasses of IOException without changing their method signature and callers are not likely to recover from them. This is unusual for a Java project and defeats the purpose of how Java checked exceptions are designed to work.
Effective Java 2nd edition item 61 "Throw exceptions appropriate to the abstraction" has more advice on this subject. It's likely that in a some places where we currently throw IOException, we should be wrapping in a higher-level custom exception like ObjectAccessException. Alternatively, we can let IOException bubble through for low-level I/O errors and use a higher-level custom exception for higher-level errors like incorrect object type.
Possible actions:
1. Deprecate methods that swallow and hide I/O errors and adapt callers to use replacements for them. I've wished for this for a long time; sorry I didn't act on it for so long.
2. Come up with a new proposed exception hierarchy for JGit exceptions and a migration path to using it. Would this be worth the migration pain? Does anyone have experience with making such changes in other libraries?
Other thoughts?
Thanks,
Jonathan
