Enhancements to FitbitException

[mxa0079]

Should FitbitException contain an HttpResponseMessage property?

Any other enhancements? Maybe auto-filling the List<ApiErros> based on the contents of HttpResponseMessage

Thoughts?

[WestDiscGolf]

And move the ...

                try
                {
                    var serializer = new JsonDotNetSerializer { RootProperty = "errors" };
                    errors.AddRange(serializer.Deserialize<List<ApiError>>(await response.Content.ReadAsStringAsync()));
                }
                catch
                {
                    // if there is an error with the serialization then we need to default the errors back to an instantiated list
                    errors = new List<ApiError>();
                }  

from the HandleResponse into the Exception?

Be interesting to investigate if that would look right. My only concern, without trying it, would be the Exceptions are then doing a reasonable amount of logic and I'm not sure how I feel about Exceptions doing that.

[WestDiscGolf]

Seem to be doing this while I do #106 :-)

[WestDiscGolf]

See PR https://github.com/aarondcoleman/Fitbit.NET/pull/121

[mxa0079]

I like the update in PR #121 -- exceptions now give us access to the list of errors thrown by Fitbit (although I still think that the multiple errors per response is an overhead.... they should just reply back with a single error at a time).

However, I think we still need to also provide access to the response's HttpStatusCode as part of the exception.

@aarondcoleman @WestDiscGolf thoughts?

[WestDiscGolf]

@mxa0079 If you look at my FitbitException re-work you'll notice the base exception no longer has a HttpStatusCode as not all exceptions from the library relate to a response back from the API such as the parsing error one or stale token one (I think without looking). So if an exception is due to communication it has a response code.

[mxa0079]

It is my understanding that all API request errors will have a HttpStatusCode other than 200. Hence, I believe it would be very valuable to surface this information through the exception for a cleaner error handling experience.

If the error happens in the network, then we should not be throwing a FitbitException, but the appropriate framework exception.

However, I have not spent much time on this part of the code base so I may be making some incorrect assumptions.

[WestDiscGolf]

I agree that an issue with the API should have a HttpStatusCode which corresponds to it, and in the re-work it does, however not all exceptions the library raises are to do with communication. I see the FitbitException and derived types as the exceptions that are library specific and not communication with the Fitbit API end points specific.

[mxa0079]

Moving this discussion for Post V2.0 as we do not see us implementing the HttpStatusCode before then. This can be a backward compatible change and therefore part of a 2.X update.