Jason Dobies wrote:
Jesus Rodriguez wrote:
This is more of a note to self, but I'd figured I'd share with everyone.
We need to revisit our exception classes, the api has a very rich set
of exceptions that can be useful by other parts of the code.

Most folks don't want to use them because they live in an xmlrpc package, and we could probably move them to a more common location.
Another reason is not all of the api exceptions are localized (usually
the older ones).

Another thing I noticed was that the exceptions that are localized
none of them use the FaultExceptions constructor which accepts the
localized parameters:

public FaultException(int error, String lbl, String messageId, Object [] args) { super(LocalizationService.getInstance().getMessage(messageId, args));
    this.errorCode =  error;
    this.label =  lbl;
  }

We seem to do it the hardway :)
  ...
  super(1062, "Invalid Arch" , LocalizationService.getInstance().
          getMessage("api.system.invalidarch", new Object [] {arch}));

This could've been easily written as:

super(1062, "Invalid Arch", "api.system.invalidarch", new Object [] {arch}));

Honestly, I would do away with the messageid altogether and use
the exception label as the messageid. So the ctor would change to:

  super(1062, "Invalid Arch", new Object [] {arch}));

And in FaultException (top example) it would change to this:

  public FaultException(int error, String lbl, Object [] args) {
    super(LocalizationService.getInstance().getMessage(lbl, args));
    this.errorCode =  error;
    this.label =  lbl;
  }

Oh back to the organization, I think we should move the Exceptions to
com.redhat.rhn.common.errors. except for maybe the truly specific
errors which could remain in the xmlrpc package.

Thoughts?


My only concern is that our naming has been... less than stellar. If we put all exceptions in the same package, it may be more difficult to discern which exception applies to a particular usage.

The natural next reaction is that it will actually be a good thing, forcing us to revisit our exceptions as a whole. I'm for that, but we just need to make sure we allocate that time with this move.

Regarding the exceptions, I agree that their handling could be improved. This includes naming, localizing, making them more 'common' so that they can better leveraged by both API and UI (e.g. perhaps under rhn.common)...etc. I don't think this is a small effort overall, but could lead to better consistency in the errors between the API and UI wrt what the user sees and make it easier for a developer to 'find' and resuse exceptions...etc.

_______________________________________________
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Reply via email to