0

Currently my signup looks like this:

public void signup(User newUser) throws Exception {
    log.info("Sign up: " + newUser.getEmail());
    if (restService.emailAlreadyExists(newUser.getEmail())) {
        throw new Exception("Email already in use.");
    }
    List<Role> roles = new ArrayList<Role>();
    roles = roleRepository.findAllOrderedByName();
    roles.add(roleRepository.findByName("user"));
    newUser.setRoles(roles);
    newUser.setPassword(restService.getHashedValue(newUser.getPassword()));
    try {
        em.persist(newUser);
    } catch (Exception e) {
        throw new Exception("Just noobs use apps with less bugs. Try again. Now!");
    }
    log.info(newUser.toString());
    userEvent.fire(newUser);
}

In first order I'm just interested in two messages (will become FacesMessage) for the user. To prevent other cryptic messages for the user, I even would need to extend the try-block up to roles.

Well, that would be bad practice, I guess. Also using a generic Exception smells, they say. But: I detect following documented Exceptions in this small piece of code:

  • IllegalStateException
  • IllegalArgumentException
  • EntityExistsException
  • TransactionRequiredException
  • ObserverException

Even not speaking about the eight(!) Exceptions of the method getSingleResult() of javax.persistence.TypedQuery.

Should I really handle all Exceptions in this example, or is it ok to skip a few (and/or maybe even use a generic Exception like above).

Martin
  • 239
  • 1
  • 15
  • 2
    What you should ask yourself is: should you handle them all, and how? A lot of UncheckedExceptions represent an error the system can not recover from. So, how will you handle this, without blocking the UI and annoying the user? A number of them can be handled, but if there's nothing the user or the system can do to correct the problem, should you? – Stultuske Apr 07 '16 at 07:55
  • Also: let's say you write the back-end. Do you know how the UI is supposed to respond on an Exception? From time to time, propagating the exception is better than assuming you know what to do. – Stultuske Apr 07 '16 at 07:56
  • See http://stackoverflow.com/questions/2416316/why-is-the-catchexception-almost-always-a-bad-idea – Raedwald Apr 07 '16 at 08:52
  • Possible duplicate of [Can I catch multiple Java exceptions in the same catch clause?](http://stackoverflow.com/questions/3495926/can-i-catch-multiple-java-exceptions-in-the-same-catch-clause) – Raedwald Apr 07 '16 at 08:53
  • The two examples of your possible duplicates: The first one is just about generic or not, and the second about the code-possibilities to catch several Exceptions). I ask in first order: Should I even handle them (e.g. the ones from roleRepository), and when, then really all? – Martin Apr 07 '16 at 09:19
  • Furthermore: https://docs.oracle.com/javase/tutorial/essential/exceptions/catch.html says: Note: If a catch block handles more than one exception type, then the catch parameter is implicitly final. In this example, the catch parameter ex is final and therefore you cannot assign any values to it within the catch block. And I need to throw at least two because of the custom messages. – Martin Apr 07 '16 at 09:34

1 Answers1

0

Best practice is: catch all exceptions separately and make as more custom log messages as possible. It's easier to understand faulty situations and to act accordingly.

Reality in a typical business application is: try to group your exceptions (ex. group all possible exceptions caused by your method input, all the ones throwed by the db etc. etc.) and learn that sometimes you will put the infamous catch (Exception e) or rethrow a generic exception with a generic message... or people will start calling you about logs growing like elephants.

Matteo Baldi
  • 5,613
  • 10
  • 39
  • 51
  • 3
    I thought best practice was to throw as few exceptions as possible. Catching every possible exception is a fruitless task IMHO – Bohemian Apr 07 '16 at 08:05
  • In my case that would be no Exception (all unchecked, beside the two ones I want to throw anyway because of the message). And then it would also not matter, that it is a generic Exception, isn't it? – Martin Apr 07 '16 at 09:13