Dwolla / dwolla-v2-csharp

Official C# Wrapper for Dwolla's API
https://developers.dwolla.com
MIT License
12 stars 15 forks source link

Error Detection Supersedes Valid Content Response #47

Closed dahlbyk closed 1 year ago

dahlbyk commented 1 year ago

https://developers.dwolla.com/guides/business-verified-customer/handle-verification-statuses#determining-verification-documents example response documents a scenario where errors is expected along with valid content and a 200 OK response body.

This regex matches on that response, suppressing valid Content (important to be able to repair the Customer) in the presence of errors: https://github.com/Dwolla/dwolla-v2-csharp/blob/393fd173bdf88e48b49d5c7ffdfcb4535dbe846d/Dwolla.Client/Rest/ResponseBuilder.cs#L29-L31

Expected behavior: either...

Actual behavior:

ShreyaThapa commented 1 year ago

Hi @dahlbyk -- thanks for reporting! We were table to test internally and confirm that this is a bug. The expected behavior is that the Content should always be populated for 200 Ok response even if it contains an error object.

We have logged a bug ticket on our end to work on a fix, however, we do not anticipate to have it rolled out this week or the next.

In the meantime, feel free to clone the repo and update the logic so that you can continue using the SDK in your application. We also appreciate pull requests if you would like to contribute to the repo!

Thank you and Happy Holidays!

dahlbyk commented 1 year ago

We were able to patch and work around this: https://github.com/Stratafolio/dwolla-v2-csharp/tree/gh-47-hack.

But a more correct fix seems like it would need to add _embedded to BaseResponse so errors are available in the successful response.

ShreyaThapa commented 1 year ago

Thanks @dahlbyk!

Logging the proposed two-fold solution for reference:

ShreyaThapa commented 1 year ago

This issue has been fixed in the latest release 5.4.0.

dahlbyk commented 1 year ago

Great, thanks!

(Fixed in #48)