akamsteeg / AtleX.HaveIBeenPwned

A modern, fully async, high performance, .NET client library for the API of HaveIBeenPwned.com
https://www.nuget.org/packages/AtleX.HaveIBeenPwned/
MIT License
5 stars 0 forks source link

Throw custom exception when an invalid or expired API key is used #28

Closed akamsteeg closed 5 years ago

akamsteeg commented 5 years ago

When an invalid or expired API key is used, the service returns a 401. Currently, we create a HaveIBeenPwnedClientException with a message containing the response code and phrase. That's a bit of a leaky abstraction. The user doesn't have to know anything about the internals of the lib, the HaveIBeenPwned.com API or the transport. This is a common use case, so we should handle it in HaveIBeenPwnedClient.HandleErrorResponse(HttpResponseMessage) by returning either a HaveIBeenPwnedClientException with a custom clear error message or a custom exception type.

If we go for a custom exception, we should let it inherit from HaveIBeenPwnedClientException so it will not be a breaking change and catching exceptions generated by the lib is also easier. All custom exceptions (e.g. RateLimitExceededException) inherit from HaveIBeenPwnedClientException so a catch (HaveIBeenPwnedClientException ex) is easy for the end user.

akamsteeg commented 5 years ago

I've chosen to go with a custom exception type for this, so I introduced a InvalidApiKeyException. It inherits from HaveIBeenPwnedClientException so it's still catchable in a generic exception handler for HaveIBeenPwnedClientException or Exception. This doesn't break any 4.0.0 clients currently catching HaveIBeenPwnedClientExceptions.

Clients catching HaveIBeenPwnedClientExceptions or Exceptions and then comparing exception messages will break. However, exception messages are not a recommended thing to act on (they can be localized, for example) and someone doing this is already in a world of pain. A bit more shouldn't and wouldn't matter, so we can safely throw this exception.