Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error Detection Supersedes Valid Content Response #47

Closed
dahlbyk opened this issue Dec 21, 2022 · 5 comments
Closed

Error Detection Supersedes Valid Content Response #47

dahlbyk opened this issue Dec 21, 2022 · 5 comments

Comments

@dahlbyk
Copy link

dahlbyk commented Dec 21, 2022

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:

return ErrorRegex.IsMatch(rawContent)
? Error<T>(response, JsonConvert.DeserializeObject<ErrorResponse>(rawContent), rawContent)
: new RestResponse<T>(response, JsonConvert.DeserializeObject<T>(rawContent), rawContent);

Expected behavior: either...

  • 200 OK response from GET Customer always deserializes the Customer into RestResponse<Customer>.Content.
  • 200 OK API response shouldn't include errors. In this case, the DBA required error is also communicated via _links['upload-dba-document'].

Actual behavior:

  • 200 OK response from GET Customer returns a RestResponse<Customer> with Content = null.
  • 200 OK response includes errors, which I don't recall seeing for other document-required scenarios.
@ShreyaThapa
Copy link
Contributor

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
Copy link
Author

dahlbyk commented Jan 3, 2023

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
Copy link
Contributor

Thanks @dahlbyk!

Logging the proposed two-fold solution for reference:

  • add _embedded to BaseResponse
  • populate Content for all 200 Ok responses including ones that have an error object

@ShreyaThapa
Copy link
Contributor

This issue has been fixed in the latest release 5.4.0.

@dahlbyk
Copy link
Author

dahlbyk commented Feb 9, 2023

Great, thanks!

(Fixed in #48)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants