Big exception refactor and simplification (#11)

* Big exception refactor and simplification
* fix test coverage

Author: jpstroop

docs/VALIDATIONS_AND_EXCEPTIONS.md

@@ -1,10 +1,16 @@
 # Input Validation and Error Handling
 
-Many method parameter arguments are validated before making API requests. The
-aim is to encapulate the HTTP API as much as possible and raise more helpfule
-exceptions before a bad request is executed. Understanding these validations and
-the exceptions that are raised by them (and elsewhere) will help you use this
-library correctly.
+Many method parameter arguments are validated **before making any API
+requests**. The aim is to encapsulate the HTTP API as much as possible and raise
+more helpful exceptions before a bad request is executed. This approach:
+
+- Preserves your API rate limits by catching errors locally
+- Provides more specific and helpful error messages
+- Simplifies debugging by clearly separating client-side validation issues from
+  API response issues
+
+Understanding these validations and the exceptions that are raised by them (and
+elsewhere) will help you use this library correctly and efficiently.
 
 ## Input Validation
 
@@ -167,9 +173,52 @@ except ValidationException as e:
 
 ## Exception Handling
 
-There are many custom exceptions, When validation fails or other errors occur,
+There are many custom exceptions. When validation fails or other errors occur,
 the library raises specific exceptions that help identify the problem.
 
+### Using Custom Validation Exceptions
+
+Client validation exceptions (`ClientValidationException` and its subclasses)
+are raised *before* any API call is made. This means:
+
+1. They reflect problems with your input parameters that can be detected locally
+2. No network requests have been initiated when these exceptions occur
+3. They help you fix issues before consuming API rate limits
+
+This is in contrast to API exceptions (`FitbitAPIException` and its subclasses),
+which are raised in response to errors returned by the Fitbit API after a
+network request has been made.
+
+When using this library, you'll want to catch the specific exception types for
+proper error handling:
+
+```python
+from fitbit_client.exceptions import ParameterValidationException, MissingParameterException
+
+try:
+    # When parameters might be missing
+    client.nutrition.create_food_goal(calories=None, intensity=None)
+except MissingParameterException as e:
+    print(f"Missing parameter: {e.message}")
+
+try:
+    # When parameters might be invalid
+    client.sleep.create_sleep_goals(min_duration=-10)
+except ParameterValidationException as e:
+    print(f"Invalid parameter value for {e.field_name}: {e.message}")
+```
+
+You can also catch the base class for all client validation exceptions:
+
+```python
+from fitbit_client.exceptions import ClientValidationException
+
+try:
+    client.activity.create_activity_log(duration_millis=-100, start_time="12:00", date="2024-02-20")
+except ClientValidationException as e:
+    print(f"Validation error: {e.message}")
+```
+
 ### ValidationException
 
 Raised when input parameters do not meet requirements:
@@ -238,17 +287,49 @@ except RateLimitExceededException as e:
 
 ### Exception Properties
 
-All exceptions provide these properties:
+API exceptions (`FitbitAPIException` and its subclasses) provide these
+properties:
 
 - `message`: Human-readable error description
 - `status_code`: HTTP status code (if applicable)
 - `error_type`: Type of error from the API
 - `field_name`: Name of the invalid field (for validation errors)
 
+Validation exceptions (`ClientValidationException` and its subclasses) provide:
+
+- `message`: Human-readable error description
+- `field_name`: Name of the invalid field (for validation errors)
+
+Specific validation exception subclasses provide additional properties:
+
+- `InvalidDateException`: Adds `date_str` property with the invalid date string
+- `InvalidDateRangeException`: Adds `start_date`, `end_date`, `max_days`, and
+  `resource_name` properties
+- `IntradayValidationException`: Adds `allowed_values` and `resource_name`
+  properties
+- `ParameterValidationException`: Used for invalid parameter values (e.g.,
+  negative where positive is required)
+- `MissingParameterException`: Used when required parameters are missing or
+  parameter combinations are invalid
+
 ### Exception Hierarchy:
 
 ```
 Exception
+├── ValueError
+│   └── ClientValidationException             # Superclass for validations that take place before 
+│       │                                     #   making a request
+│       ├── InvalidDateException              # Raised when a date string is not in the correct 
+│       │                                     #   format or not a valid calendar date
+│       ├── InvalidDateRangeException         # Raised when a date range is invalid (e.g., end is 
+│       │                                     #   before start, exceeds max days)
+│       ├── PaginationException               # Raised when pagination parameters are invalid
+│       ├── IntradayValidationException       # Raised when intraday request parameters are invalid
+│       ├── ParameterValidationException      # Raised when a parameter value is invalid
+│       │                                     #   (e.g., negative when positive required)
+│       └── MissingParameterException         # Raised when required parameters are missing or
+│                                             #   parameter combinations are invalid
+│
 └── FitbitAPIException                        # Base exception for all Fitbit API errors
     │
     ├── OAuthException                        # Superclass for all authentication flow exceptions
@@ -257,23 +338,15 @@ Exception
     │   ├── InvalidTokenException             # Raised when the OAuth token is invalid
     │   └── InvalidClientException            # Raised when the client_id is invalid
     │
-    ├── RequestException                      # Superclass for all API request exceptions
-    │   ├── InvalidRequestException           # Raised when the request syntax is invalid
-    │   ├── AuthorizationException            # Raised when there are authorization-related errors
-    │   ├── InsufficientPermissionsException  # Raised when the application has insufficient permissions
-    │   ├── InsufficientScopeException        # Raised when the application is missing a required scope
-    │   ├── NotFoundException                 # Raised when the requested resource does not exist
-    │   ├── RateLimitExceededException        # Raised when the application hits rate limiting quotas
-    │   ├── SystemException                   # Raised when there is a system-level failure
-    │   └── ValidationException               # Raised when a request parameter is invalid or missing
-    │
-    └── ClientValidationException             # Superclass for validations that take place before 
-        │                                     #   making a request
-        ├── InvalidDateException              # Raised when a date string is not in the correct 
-        │                                     #   format or not a valid calendar date
-        ├── InvalidDateRangeException         # Raised when a date range is invalid (e.g., end is 
-        │                                     #   before start, exceeds max days)
-        └── IntradayValidationException       # Raised when intraday request parameters are invalid
+    └── RequestException                      # Superclass for all API request exceptions
+        ├── InvalidRequestException           # Raised when the request syntax is invalid
+        ├── AuthorizationException            # Raised when there are authorization-related errors
+        ├── InsufficientPermissionsException  # Raised when the application has insufficient permissions
+        ├── InsufficientScopeException        # Raised when the application is missing a required scope
+        ├── NotFoundException                 # Raised when the requested resource does not exist
+        ├── RateLimitExceededException        # Raised when the application hits rate limiting quotas
+        ├── SystemException                   # Raised when there is a system-level failure
+        └── ValidationException               # Raised when a request parameter is invalid or missing
 ```
 
 ## Debugging

fitbit_client/auth/callback_server.py

@@ -54,7 +54,7 @@ def __init__(self, redirect_uri: str) -> None:
             raise InvalidRequestException(
                 message="Request to invalid domain: redirect_uri must use HTTPS protocol.",
                 status_code=400,
-                error_type="request",
+                error_type="invalid_request",
                 field_name="redirect_uri",
             )
 
@@ -237,9 +237,10 @@ def wait_for_callback(self, timeout: int = 300) -> Optional[str]:
 
         self.logger.error("Callback wait timed out")
         raise InvalidRequestException(
-            message="OAuth callback timed out waiting for response",
+            message=f"OAuth callback timed out after {timeout} seconds",
             status_code=400,
             error_type="invalid_request",
+            field_name="oauth_callback",
         )
 
     def stop(self) -> None:

fitbit_client/auth/oauth.py

@@ -73,7 +73,8 @@ def __init__(
             raise InvalidRequestException(
                 message="This request should use https protocol.",
                 status_code=400,
-                error_type="request",
+                error_type="invalid_request",
+                field_name="redirect_uri",
             )
 
         environ["OAUTHLIB_INSECURE_TRANSPORT"] = "1"
@@ -128,7 +129,14 @@ def _load_token(self) -> Optional[TokenDict]:
                     except InvalidGrantException:
                         # Invalid/expired refresh token
                         return None
-        except Exception:
+        except json.JSONDecodeError:
+            self.logger.error(f"Invalid JSON in token cache file: {self.token_cache_path}")
+            return None
+        except OSError as e:
+            self.logger.error(f"Error reading token cache file: {self.token_cache_path}: {str(e)}")
+            return None
+        except Exception as e:
+            self.logger.error(f"Unexpected error loading token: {e.__class__.__name__}: {str(e)}")
             return None
         return None
 
@@ -139,7 +147,23 @@ def _save_token(self, token: TokenDict) -> None:
         self.token = token
 
     def authenticate(self, force_new: bool = False) -> bool:
-        """Complete authentication flow if needed"""
+        """Complete authentication flow if needed
+
+        Args:
+            force_new: Force new authentication even if valid token exists
+
+        Returns:
+            bool: True if authenticated successfully
+
+        Raises:
+            InvalidRequestException: If the request syntax is invalid
+            InvalidClientException: If the client_id is invalid
+            InvalidGrantException: If the grant_type is invalid
+            InvalidTokenException: If the OAuth token is invalid
+            ExpiredTokenException: If the OAuth token has expired
+            OAuthException: Base class for all OAuth-related exceptions
+            SystemException: If there's a system-level failure
+        """
         if not force_new and self.is_authenticated():
             self.logger.debug("Authentication token exchange completed successfully")
             return True
@@ -164,18 +188,9 @@ def authenticate(self, force_new: bool = False) -> bool:
             callback_url = input("Enter the full callback URL: ")
 
         # Exchange authorization code for token
-        try:
-            token = self.fetch_token(callback_url)
-            self._save_token(token)
-            return True
-        except Exception as e:
-            if "invalid_grant" in str(e):
-                raise InvalidGrantException(
-                    message="Authorization code expired or invalid",
-                    status_code=400,
-                    error_type="invalid_grant",
-                ) from e
-            raise
+        token = self.fetch_token(callback_url)
+        self._save_token(token)
+        return True
 
     def is_authenticated(self) -> bool:
         """Check if we have valid tokens"""
@@ -192,7 +207,21 @@ def get_authorization_url(self) -> Tuple[str, str]:
         return (str(auth_url_tuple[0]), str(auth_url_tuple[1]))
 
     def fetch_token(self, authorization_response: str) -> TokenDict:
-        """Exchange authorization code for access token"""
+        """Exchange authorization code for access token
+
+        Args:
+            authorization_response: The full callback URL with authorization code
+
+        Returns:
+            TokenDict: Dictionary containing access token and other OAuth details
+
+        Raises:
+            InvalidClientException: If the client credentials are invalid
+            InvalidTokenException: If the authorization code is invalid
+            InvalidGrantException: If the authorization grant is invalid
+            ExpiredTokenException: If the token has expired
+            OAuthException: For other OAuth-related errors
+        """
         try:
             auth = HTTPBasicAuth(self.client_id, self.client_secret)
             token_data = self.session.fetch_token(
@@ -208,31 +237,54 @@ def fetch_token(self, authorization_response: str) -> TokenDict:
         except Exception as e:
             error_msg = str(e).lower()
 
-            if "invalid_client" in error_msg:
-                self.logger.error(
-                    f"InvalidClientException: Authentication failed "
-                    f"(Client ID: {self.client_id[:4]}..., Error: {str(e)})"
-                )
-                raise InvalidClientException(
-                    message="Invalid client credentials",
-                    status_code=401,
-                    error_type="invalid_client",
-                ) from e
-            if "invalid_token" in error_msg:
-                self.logger.error(
-                    f"InvalidTokenException: Token validation failed " f"(Error: {str(e)})"
-                )
-                raise InvalidTokenException(
-                    message="Invalid authorization code",
-                    status_code=401,
-                    error_type="invalid_token",
-                ) from e
+            # Use standard error mapping from ERROR_TYPE_EXCEPTIONS
+            # Local imports
+            from fitbit_client.exceptions import ERROR_TYPE_EXCEPTIONS
+            from fitbit_client.exceptions import OAuthException
+
+            # Check for known error types
+            for error_type, exception_class in ERROR_TYPE_EXCEPTIONS.items():
+                if error_type in error_msg:
+                    # Special case for client ID to mask most of it in logs
+                    if error_type == "invalid_client":
+                        self.logger.error(
+                            f"{exception_class.__name__}: Authentication failed "
+                            f"(Client ID: {self.client_id[:4]}..., Error: {str(e)})"
+                        )
+                    else:
+                        self.logger.error(
+                            f"{exception_class.__name__}: {error_type} error during token fetch: {str(e)}"
+                        )
 
-            self.logger.error(f"OAuthException: {e.__class__.__name__}: {str(e)}")
-            raise
+                    raise exception_class(
+                        message=str(e),
+                        status_code=(
+                            401 if "token" in error_type or error_type == "authorization" else 400
+                        ),
+                        error_type=error_type,
+                    ) from e
+
+            # If no specific error type found, use OAuthException
+            self.logger.error(
+                f"OAuthException during token fetch: {e.__class__.__name__}: {str(e)}"
+            )
+            raise OAuthException(message=str(e), status_code=400, error_type="oauth") from e
 
     def refresh_token(self, refresh_token: str) -> TokenDict:
-        """Refresh the access token"""
+        """Refresh the access token
+
+        Args:
+            refresh_token: The refresh token to use
+
+        Returns:
+            TokenDict: Dictionary containing new access token and other OAuth details
+
+        Raises:
+            ExpiredTokenException: If the access token has expired
+            InvalidGrantException: If the refresh token is invalid
+            InvalidClientException: If the client credentials are invalid
+            OAuthException: For other OAuth-related errors
+        """
         try:
             auth = HTTPBasicAuth(self.client_id, self.client_secret)
             extra = {
@@ -248,12 +300,29 @@ def refresh_token(self, refresh_token: str) -> TokenDict:
             return token
         except Exception as e:
             error_msg = str(e).lower()
-            if "expired_token" in error_msg:
-                raise ExpiredTokenException(
-                    message="Access token expired", status_code=401, error_type="expired_token"
-                ) from e
-            if "invalid_grant" in error_msg:
-                raise InvalidGrantException(
-                    message="Refresh token invalid", status_code=400, error_type="invalid_grant"
-                ) from e
-            raise
+
+            # Use standard error mapping from ERROR_TYPE_EXCEPTIONS
+            # Local imports
+            from fitbit_client.exceptions import ERROR_TYPE_EXCEPTIONS
+            from fitbit_client.exceptions import OAuthException
+
+            # Check for known error types
+            for error_type, exception_class in ERROR_TYPE_EXCEPTIONS.items():
+                if error_type in error_msg:
+                    self.logger.error(
+                        f"{exception_class.__name__}: {error_type} error during token refresh: {str(e)}"
+                    )
+
+                    raise exception_class(
+                        message=str(e),
+                        status_code=(
+                            401 if "token" in error_type or error_type == "authorization" else 400
+                        ),
+                        error_type=error_type,
+                    ) from e
+
+            # If no specific error type found, use OAuthException
+            self.logger.error(
+                f"OAuthException during token refresh: {e.__class__.__name__}: {str(e)}"
+            )
+            raise OAuthException(message=str(e), status_code=400, error_type="oauth") from e