PaySimple / PaySimpleSDK

.NET SDK for PaySimple API v4
Other
4 stars 1 forks source link

PaySimpleSdk.Helpers.WebServiceRequest eats exceptions #19

Closed RowdyRaider2003 closed 7 years ago

RowdyRaider2003 commented 7 years ago

In the private method: private async Task DoRequestAsync(HttpRequestMessage request)

when I submit a request to the Pay Simple API and get a 401 response. The subsequent call to the method: private static string GetResponseString(Stream response) inside the catch block of DoRequestAsync causes another exception. The result is that an AggregateException ends up getting thrown to my code which calls the PaySimpleSDK. The InnerException(s) property of the Aggregate exception contains a ObjectDisposedException with the message "Cannot access a closed Stream."

The impact is that trouble shooting our system which calls the Pay Simple API is unable to log a useful error.

Here is a screen snip from a VS 2017 Enterprise debug session: realerroreaten

BDLefebvre commented 7 years ago

@RowdyRaider2003 Looks like there's a couple issues here. We are throwing a null ref exception when we try to deserialize the api errors, and we are disposing the stream when deserializing. I don't believe any actual errors get returned for a 401, so you're actual issue lies in your api credentials. I'll have this fixed shortly.

RowdyRaider2003 commented 7 years ago

Assuming I did not fat finger my credentials I agree that the 401 can be resolved easily by activating my account etc.

The issue with this code though is it leaves my system in a place where I am unable to capture the actual errors in a production environment. Having to run the code in debug mode in visual studio is less than ideal for trouble shooting in prod.

It would seem to me that teaching the WebServiceRequest class to not choke on non successful status codes if no errors are present would allow it to then throw the real exception to my code.

I would happily create a pull request if that would help clarify my ideas?