Simplified the exception hierarchy and refactor.

[sima-zhu]

This PR addressed the issue #3503.

Major changes:

1. Simplified the hierarchy.

AzureException (base one)
     - ServiceRequestException. (Error occur when attempt to connect with azure service)
     - ServiceReponseException. (Client failed to understand the response, no status code, connection timeout)
     - HttpResponseException (Http response with status code to understand the error)
          - DecodeException 
          - ResourceExistException
          - ResourceNotFoundException
          - ClientAuthenticationException
          - ResourceModifiedException
          - TooManyRedirectsException

2. Removed two exceptions:
   ServerException (status code of 5XX). It deleted because the client does not need to distinguish different server issue (501 or 503), they can just catch the base one HttpResponseException, and log details if they care. Another reason is it is independent one, so if the client actually needed, it does not take too much efforts to add it in.
   ReadTimeoutException: This one has the same functionality as ServiceResponseException. (Delete to keep hierarchy clean)
3. Moved one exception:
   DecodeException: move to HttpResponseException (Originally under ServiceResponseException). Decode one tend to catch exception with status code, get the right response format which cannot parse or understand by client.
4. Suppress the Spotbugs checking for non-serialization fields.
5. Refactor the codes affected.

[azuresdkci]

Can one of the admins verify this patch?

[JonathanGiles]

The confusing aspect for me is that ServiceRequestException and ServiceResponseException both are HTTP-specific, but they do not have the 'Http' prefix, which the HttpResponseException does. In fact, this discrepancy makes it quite hard to parse what a user should expect - a ServiceResponseException and a HttpResponseException both contain HttpResponse instances.

To me it feels like ServiceResponseException and DecodeException overlap. I wonder if it instead makes sense to remove ServiceResponseException and to re-introduce ReadTimeoutException as a sub-type of HttpResponseException?

[sima-zhu]

@JonathanGiles

1. ServiceRequestException v.s. ServiceResponseException I have raised the question to Johan. Quote from Anna's comments:

The errors currently here are very http-centric, and I think that's fine. But having a common AzureError at the top that we can reuse for AMQP-specific types is also useful.
The parent AzureError has plenty of common attributes that could be shared between the next tier of protocol specific types

I don't have a strong point against the naming, so I followed their definition.
2. For ServiceResponseException, HttpResponse is not required field, as there is no response returned in some cases. I prefer to align with Python if no strong points to decline. One thing to do, I can remove the HttpResponse if it is so hard to distinguish between ServiceResponseException and HttpResponseException.
3. ServiceResponseException v.s. DecodeException ServiceResponseException has two major jobs (including some other satisfied the JavaDoc):(ReadTimeout and DecodeError) The DecodeException here emphasize more on the http response with status code. Also, we got the full response, but the deserialization failed due to lack of some config. ServiceResponseException does not ask for status code and the deserialization error cause by wrong format.
I have put these details in JavaDoc. One of the open scenario net yet to decide which one to throw listed in issue #3591

If we have strong points of implementing in another way, we can coordinate with Python team and then make changes in both pkgs. Let me know your thoughts.

[johanste]

I would not expect a ServiceResponseException to include a HTTP response. A HTTP request, on the other hand, yes, I would expect it to include that.

[sima-zhu]

Agreed to remove the HttpResponse field from ServiceResponseException.

[sima-zhu]

The ServiceRequestException currently takes in HttpRequest. I added in convenient to people who wants to take a peek at the bad request. Let me know your thoughts on the field as well.

@JonathanGiles @johanste

[JonathanGiles]

The ServiceResponseException class was changed to not contain the HttpResponse information. It now feels a little odd that the ServiceRequestException includes the HttpRequest information. What do you think?

[sima-zhu]

I am ok to remove this out. I added this for checking what the failed request is. I can remove for now, since it is not that hard to add it back if needed.

[JonathanGiles]

My inclination, rather than remove the HttpRequest field, would have been to rename the ServiceRequestException class to instead be HttpRequestException.

[sima-zhu]

Have sent out an email to lock down the design. Will wait for the response and sign off before implementation here.