Smells in exception handling: how to separate between public and internal exceptions?

Question

During the development of a small web API, we decided to separate internal exceptions from public exceptions. Public exceptions are HTTP exceptions, that translate into HTTP responses with proper status code (e.g: BadRequestHttpException, NotFoundHttpException, etc). Internal exceptions are either exceptions thrown from adapters in our system (cache layer, persistence layer, etc.) or the domain (InvalidUserException, BadPasswordException, etc.).

This has been great so far. However, our controllers are starting to smell really bad:

    try {
        securityService.login(credentials);
    } catch (InvalidOAuthProviderException e) {
        throw new BadRequestHttpException(e.getMessage(), e.getCode());
    } catch (UserNotFoundException e) {
        throw new NotFoundHttpException(e.getMessage(), e.getCode());
    } catch (InactiveUserException e) {
        throw new NotFoundHttpException(e.getMessage(), e.getCode());
    } catch (InvalidOAuthTokenException e) {
        throw new ForbiddenHttpException(e.getMessage(), e.getCode());
    }

The HttpException constructor signature is the error message and code. This will result in a JSON body: {"message":"","code":222}. The code is the unique error code for doc reference. The actual response HTTP code is defined by the type of the exception.

This is clearly not a good sign and i'm sure there's something wrong in the design here. Any help?

Does e.getCode() return an HTTP status code? If so you've already muddied the boundaries between internal an external exceptions. — Chris Cooper
– Chris Cooper, Commented Feb 17, 2015 at 13:11
I have updated the question to make it clear. The code is our error code, for documentation reference. — Klaus S.
– Klaus S., Commented Feb 17, 2015 at 13:13

gbjbaanb · Accepted Answer · 2015-02-17 13:33:35Z

You are handling errors across the boundary between two systems, effectively, so its best to think of the http handler as a system and the internal logic that throws these internal exceptions as a library.

Given you want to catch these at the boundary point, and you do not want to map internal to external exceptions, then you're left with a simple catch-all (or a couple of catch-most) handlers. The client doesn't want to know about the internal exception, so catching everything from a login request and returning a forbidden or bad request exception is the way to go, considering you include the error message from the internal exception, it doesn't really matter what the internal exception was as far as the client interface is concerned - something went wrong, it was either a bad user or some server error.

In most cases, you should be catching the internal errors and performing some other form of handler to them - like logging them or raising an admin alarm, and then returning a more generic "it didn't work" exception to the client. The only difference here is for exceptions you know are 'expected' such as login failure due to a bad password.

Gondy · Accepted Answer · 2015-02-17 13:44:55Z

0

You can create reusable utility class that will resolve correct HttpException.

try {
        securityService.login(credentials);
    } catch (InternalException e) {
        throw ExceptionUtil.resolveHttpException(e);
    }      

public class ExceptionUtil {
  public static HttpException resolveHttpException(InternalException exc) {
     if (exc instanceOf InvalidOAuthProviderException) 
        return new InvalidOAuthProviderException(e.getMessage(), e.getCode()
     else if (exc instanceOf AnotherInternalException)
        return new  AnotherInternalException;
      ...
    }

}

Then you can use it across all application without code redundancy.

edited Feb 17, 2015 at 13:44

answered Feb 17, 2015 at 13:16

Gondy

1193 bronze badges

I'm not controlling flow, i'm controlling output. Depending on the kind of domain or persistence layer exception i'm getting, it's a different output: either Forbidden, NotFound, etc.

Klaus S.
– Klaus S.

2015-02-17 13:20:58 +00:00
Commented Feb 17, 2015 at 13:20
Changed answer.

Gondy
– Gondy

2015-02-17 13:45:08 +00:00
Commented Feb 17, 2015 at 13:45

Add a comment |

Stack Exchange Network

Smells in exception handling: how to separate between public and internal exceptions?

2 Answers 2

Your Answer

Hot Network Questions

Smells in exception handling: how to separate between public and internal exceptions?

2 Answers 2

Your Answer

Sign up or log in

Post as a guest

Related

Hot Network Questions