views:

92

answers:

2

My colleague wrote the following stackoverflow question: other stack overflow question on this topic

The question seems to have been misinterpreted and I want to find out the answer, so I'm starting this new question... hopefully a little more clear.

Basically, we have a REST API. Users of our API call our methods with parameters. But sometimes users call them with the wrong parameters!! Maybe a mistake in their code, maybe they're just trying to play with us, maybe they're trying to see how we respond, who knows! We respond with HTTP status error codes and maybe a detailed description of the invalid parameter in the XML response.

All is well. But internally we deal with these invalid parameters by throwing exceptions. For example, if someone looks up a Person object by giving us their profile id, but the profile id doesn't exist... we throw a PersonInvalidException when looking them up. Then we catch this exception in our API controller and send back an HTTP 400 status error code.

Our question is... is this the best practice, throwing exceptions internally for this kind of user error? These exceptions never get propogated back to the user, this is a REST API. They only make our code cleaner. Otherwise we could have a validation method in each of our API controllers to make sure the parameters all make sense, but that seems inefficient. We have to look up things in our database potentially twice. Or we could return nulls and check for them, but that sucks...

What are your thoughts?

A: 

I think that throwing (and then catching) exceptions internally when request parameter error is detected is perfectly fine. I've tried other approaches, but in my experience this works best.

My latest attempt at this is to do the following:

  • Define an (unchecked) exception called RequestFailureException that has an HTTP status code as one of its attributes.
  • Define subtypes of RequestFailureException for some of the common cases.
  • Define methods in my controller base class to get optional and mandatory request arguments of various types. These methods throw the relevant exceptions above.
  • Define a static handleException method in the controller base class that logs exceptions and maps them to HTTP status codes.
  • In the base class define doRequest or whatever to catch exceptions and process them using handleException.
Stephen C
I like your suggestions, thanks.
at
+1  A: 

This seems to me to be a reasonable use of exceptions; you can't expect your code to recover from user errors. The key is that what you return to the user should be detailed enough to allow them to understand (and correct) the cause of the error. Simply returning HTTP 400 or any other HTTP error code is not going to be sufficient to allow your users to determine what they are doing wrong.

ig0774
I guess you're right. Exceptions, when not exposing them to users, still bother me... but they are meant for API user errors...
at