maven-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From John Casey <jdca...@commonjava.org>
Subject Exceptions in maven-project
Date Tue, 06 Nov 2007 20:04:17 GMT
Recently, a couple of us have been making modifications to the maven- 
project exceptions, and I think it's gotten to the point where we  
need to step back and discuss what we're trying to accomplish here.  
I'll start with the changes I'm making, and a couple of other things  
I've noticed that I'd like some clarification on.

My own changes in these classes have focused on exposing constructors  
that only accept the specific types of exception parameters that are  
currently passed into these project exceptions. One notable instance  
of this is the ProjectBuildingException. In this case, there are  
quite a few constructors, since there are currently a lot of  
different types of exceptions that can be wrapped by a  
ProjectBuildingException. The point of this exercise is to improve  
the readability of the code, and allow someone browsing through the  
source to know what the potential exception chains actually are,  
without requiring a full static analysis of maven's entire codebase.  
It has the added benefit of forcing us to look at these exception  
chains a little bit, and figure out where we might be able to  
consolidate exception classes, subclass exceptions, and attach better  
context information at a given exception's construction. In the case  
of the ProjectBuildingException, I believe the profusion of  
constructors is bad; however, I likewise believe that the profusion  
of exception types wrapped by ProjectBuildingException is less than  
ideal. For the record, I don't mind supporting protected constructors  
to allow controlled subclassing of these exceptions, as long as we  
don't open up the potential for:

super( message, cause )

in a subclass of ProjectBuildingException. This destroys the context  
of this exception, and makes it very difficult to construct a  
coherent error message at the end of the build.

---

The other thing I've been struggling with lately is the use of  
java.net.URI internally within the ProjectBuildingException. I've  
done static analysis of maven's core, and the URI instance is not  
used intact anywhere in the code. While, taken with the super-POM's  
location, this field type might make a sort of pure sense, I don't  
see the value of incurring additional instability here. We're  
converting pom locations from (and, supposedly, to) java.io.File  
instances using information in this exception, but with the new  
changes, we're adding all sorts of new complexity in the code by  
constructing File instances so we can call toUri() on them...which in  
some cases, causes the build to tank because the file path has a  
space in it.

On top of this, we have protected (and private) constructors in this  
class just to allow support the DRY principle on two fields  
(projectId and pomUri). This means that, taken with the file  
construction used to support the (now deprecated) string-based  
constructors for pomLocation, we have two layers of indirection  
feeding a private constructor. This seems geared to avoid duplication  
of calls to set these two fields in the large number of new  
constructors, but the way I see it, it's a massively over-normalized  
approach, and it makes this class much harder to read with little or  
no corresponding benefit. IMO, until we have more than two fields to  
set, private constructors on these sorts of exceptions will only  
confuse their readability.

---

I guess maybe what we really need here is a better discussion of  
exactly how we support the user through errors. I've been thinking  
about this stuff quite a bit lately, and I realize it's not enough to  
have the information for the topmost and bottom-most exceptions in  
the chain. Instead, each place where an exception is wrapped, it's  
wrapped because we're traversing a layer in the architecture (or, it  
should be this way, in any event). This means that there is a new  
layer of context information associated with the site where the new  
wrapper is constructed, and this additional context can be critical  
in making the error easier to understand. Things like certain  
pertinent parameters, project instances that were in play when the  
error occurred, extra plugin information pertinent to the error, and  
so forth all need to be captured.

When it comes to actually traversing the tree of possible exceptions  
and constructing a coherent error message for the user, we also need  
to have some reasonable assurance that we have 90% of the possible  
exception chains accounted for. If new functionality results in new  
exception chains, we should have to *think* about the reporting  
implications of adding these new chains. To me, this means forcing  
the developer to deliberately enable the new chain by adding a  
constructor for the new sub-exception in the wrapper exception. It's  
not perfect, obviously, but it should help us remember that we need  
to add reporting code for this new variant of the wrapper.

Thoughts?


---
John Casey
Committer and PMC Member, Apache Maven
mail: jdcasey at commonjava dot org
blog: http://www.ejlife.net/blogs/john
rss: http://feeds.feedburner.com/ejlife/john



Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message