kevbite / CompaniesHouse.NET

A simple .NET client wrapper for CompaniesHouse API
MIT License
37 stars 44 forks source link

Include response headers in failed requests #181

Closed bernhard-hofmann closed 2 years ago

bernhard-hofmann commented 2 years ago

This is a feature request.

I'd like to propose a change that will include response header details in the Data property of the exception thrown for unsuccessful requests.

The calls to response.EnsureSuccessStatusCode() will throw without adding headers. In the case of a 503 or 429, the Retry-After header could be invaluable for correct behaviour on the part of the client. See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After

If you're in agreement, I'm offering to produce a PR that will either replace the call to the framework method (EnsureSuccessStatusCode) or catch it, add the necessary details, and then throw it (it does other things such as dispose the content so might be worth "piggybacking").

bernhard-hofmann commented 2 years ago

I've opted to only include specific headers that could be required as distinct values. I've raised a PR with the enhancement for your review. :)

kevbite commented 2 years ago

Thanks @bernhard-hofmann, I've merged this in and it's in the latest version of the NuGet package https://www.nuget.org/packages/CompaniesHouse/7.11.6

bernhard-dwe commented 2 years ago

Happy for you to close this issue now @kevbite. Thank you for the very swift merge and publish!