On 2018-03-08 15:24, Igor Ignatyev wrote:
Hi Erik,
Thanks for looking at this!
to avoid incompatibility, you could have just made ArtifactResolverException a
subclass of java.io.FileNotFoundException.
This is correct, but I don't think the exception should be of type
FileNotFoundException. IMO, this is something different and trying to
re-purpose an existing exception type is rarely a good idea.
it seems you forgot to add ArtifactResolverException.java file to the repo.
Doh! Added.
a minor nit: in JibArtifactManager::newInstance, you pass "Could not resolve "
+ JIB_SERVICE_FACTORY to ClassNotFoundException constructor. by the convention, the
message in CNFE is the classname.
Right, good point, fixed.
New Webrev: http://cr.openjdk.java.net/~erikj/8199352/webrev.02/
/Erik
-- Igor
On Mar 8, 2018, at 2:08 PM, Erik Joelsson <erik.joels...@oracle.com> wrote:
The Jib artifact resolver is not very good at telling us why things go wrong.
The reason is that it swallows exceptions. This patch changes the API from
throwing a FileNotFoundException, which I don't really think fits correctly in
all cases, to a new API specific exception.
I have greped for all uses of this API in the tests and changed the exception
type caught at the caller location. I verified that I didn't break anything by
compiling all the affected test classes and by running some of them for a bit.
With these changes it should be easier to diagnose problems with resolving
artifacts in the future.
Bug: https://bugs.openjdk.java.net/browse/JDK-8199352
Webrev: http://cr.openjdk.java.net/~erikj/8199352/webrev.01/index.html
/Erik
