use ADTs instead of exceptions#1258
Merged
AndreyNikiforov merged 46 commits intoicloud-photos-downloader:masterfrom Sep 22, 2025
Merged
use ADTs instead of exceptions#1258AndreyNikiforov merged 46 commits intoicloud-photos-downloader:masterfrom
AndreyNikiforov merged 46 commits intoicloud-photos-downloader:masterfrom
Conversation
Extract error handling and exception throwing logic from request() method into a separate _evaluate_response() method for better code organization and maintainability. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Instead of calling evaluate_response() from within session.request(), now explicitly call it at each location where HTTP requests are made. This provides better control over response evaluation and will help when converting exceptions to ADTs. Changes: - Made evaluate_response() public in PyiCloudSession - Added explicit evaluate_response() calls in base.py (14 locations) - Added explicit evaluate_response() calls in photos.py (6 locations) - Added evaluate_response() call in icloudpd/base.py delete_photo() - Added evaluate_response() for send_request() calls (3 locations) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Create ADT classes for response evaluation results - Modify evaluate_response to return ADTs instead of raising exceptions - Update all evaluate_response calls to use match statements - Remove exception imports from session.py in favor of ADTs - Update logging to use ADT data directly All tests pass with these changes.
…_response() - Create ResponseServiceUnavailable ADT for 503 errors - Move 503 check from session.request() to evaluate_response() - Add ResponseServiceUnavailable case handling to all evaluate_response call sites - Import PyiCloudServiceUnavailableException where needed All tests pass with these changes.
…on methods - Extract response evaluation logic into separate evaluate_response() method - Create ADTs for all response types (success, 2SA required, service errors, etc.) - Replace exception throwing in authentication methods with ADT returns - Move exception handling to top-level authenticate() method using pattern matching - Remove unnecessary try-catch blocks from methods that now use ADTs - Fix 2FA/2SA verification code handling to work with ADT pattern - Ensure all authentication flows properly handle ADT results All tests pass with the refactored implementation. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
… method - Move create_pyicloud_service_adt() to PyiCloudService as class method - Remove unused authenticate() method and skip_auth parameter - Return service as part of ADT result instead of tuple - Update authentication.py to use pattern matching with new ADTs - Remove dead code that was unreachable after exhaustive matching - Add new ADT types for service creation results that include service instance This change moves exception handling out of PyiCloudService initialization, making the authentication flow more explicit and type-safe through ADTs. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add TwoFactorAuthSuccess and TwoFactorAuthFailed ADT types - Update request_2sa(), request_2fa(), and request_2fa_web() to return ADTs - Move exception handling from individual methods to authenticator() - Use pattern matching in authenticator() to handle 2FA/2SA results - Maintain backward compatibility for 2SA with sys.exit(1) This change continues the ADT pattern refactoring, moving exception re-throwing from 2FA/2SA methods up to the authenticator level for better error handling control and type safety. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add AuthenticatorResult ADT types for authenticator return values - Update authenticator() to return ADTs instead of raising exceptions - Move exception handling to callers in base.py using pattern matching - Update test to handle ADT result instead of expecting exception - Maintain backward compatibility including sys.exit(1) for 2SA This completes the ADT pattern refactoring, moving all exception re-throwing out of the authentication functions and into their callers, providing better control flow and type safety. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Create check_and_create() class method for PhotoLibrary that returns ADT results - PhotoLibrary constructor no longer checks indexing state - Add PhotosServiceInitSuccess ADT for cleaner type safety - Remove unnecessary try-catch blocks that were silencing errors - Update all PhotoLibrary/PhotosService instantiation to use factory methods - Add TODO for future _fetch_libraries() ADT refactoring This ensures proper error propagation for network/API failures while individual library initialization failures are still gracefully handled. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Remove logging import and logger declaration - Remove logger.warning() and logger.error() calls - These logs were never visible since pyicloud_ipd uses NullHandler - All error cases are already handled via ADTs and proper error propagation The logging in photos.py was redundant - library initialization failures are handled through pattern matching and timezone conversion failures are handled by using the date as-is. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Added LibrariesFetchSuccess and LibrariesFetchFailed ADT types - Updated _fetch_libraries() to return LibrariesFetchResult instead of dict - Modified private_libraries and shared_libraries properties to handle ADT results - Better error propagation with explicit success/failure cases - Track skipped libraries with reasons (not indexed or init failed) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Convert the _fetch_folders method to return ADT results instead of raising exceptions, maintaining consistency with the refactoring pattern across the codebase. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Introduce get_album_length() method that returns AlbumLengthResult ADT instead of raising exceptions. The __len__() method is preserved for backward compatibility and delegates to get_album_length(), handling the ADT result appropriately. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Convert download_asset and PhotoAsset.download methods to return DownloadResult ADT instead of raising exceptions. Update download.py to handle the ADT result and re-raise exceptions for backward compatibility. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…DT method Replace len() calls on PhotoAlbum objects with explicit get_album_length() method calls that return ADT results. Remove the __len__ magic method from PhotoAlbum class to enforce explicit ADT handling throughout the codebase. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Convert PhotoAlbum iteration to yield ADT results (PhotoIterationSuccess, PhotoIterationFailed, PhotoIterationComplete) instead of directly yielding PhotoAsset objects. This ensures proper error handling at each iteration step. Also refactored photos_request() to return ADTs. Updated all callers including base.py and autodelete.py to handle the ADT iteration results with pattern matching. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add DeletePhotoResult ADT for delete photo operations - Change delete_photo() to return DeletePhotoResult instead of raising exceptions - Change autodelete_photos() to return AutodeleteResult instead of raising exceptions - Update core_single_run() to handle ADT returns and raise exceptions only at this level This ensures that exceptions are only raised in core_single_run() while all other functions in the call chain propagate ADTs, improving error handling consistency. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…T usage Replace AuthWithTokenFailed(exception) pattern with direct use of response ADTs (ResponseServiceNotActivated, ResponseAPIError, ResponseServiceUnavailable) in AuthenticateWithTokenResult union. This simplifies the type hierarchy by removing unnecessary exception wrapping and makes the ADT usage more consistent. Changes: - Remove AuthWithTokenFailed class definition - Update AuthenticateWithTokenResult union to include response ADTs directly - Update all pattern matching to handle specific response types instead of wrapper - Preserve same error handling behavior while improving type clarity 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Following the pattern established with AuthWithTokenFailed, replace other ADTs that simply wrap exceptions with unions that directly use response ADTs: - DeletePhotoResult now uses Response2SARequired | ResponseServiceNotActivated | etc. - PhotosRequestResult now uses response ADTs directly instead of PhotosRequestFailed - PhotoIterationResult now uses response ADTs directly instead of PhotoIterationFailed - AutodeleteResult now uses response ADTs directly instead of AutodeleteFailed This eliminates unnecessary exception wrapping layers and makes the ADT usage more consistent throughout the codebase. Pattern matching now directly handles specific response types rather than unpacking wrapped exceptions. Note: Some internal ADTs in photos.py (DownloadFailed, PhotoLibraryInitFailed, etc.) still wrap exceptions as they are internal to pyicloud_ipd and can be refactored separately if needed. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Replace albums, private_libraries, shared_libraries property usage with ADT methods - Replace photos property on PyiCloudService with get_photos_service() ADT method - All ADT results are propagated to core_single_run() where pattern matching handles errors - Exceptions are only raised in core_single_run(), maintaining ADT pattern throughout - Added comprehensive pattern matching for all service access error cases - Maintains backward compatibility through legacy properties This completes the migration to ADT-based error handling for all service properties, ensuring exceptions are only thrown at the top level in core_single_run().
- Changed download_media to use get_photos_service() instead of accessing photos property - Added proper ADT pattern matching for all response types - Handle PhotoLibraryNotFinishedIndexing by converting to ResponseServiceNotActivated - Propagate other ADT errors directly through the return chain - Maintains ADT-based error handling throughout the call chain This completes the refactoring to use ADTs instead of exceptions for the photos service access. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Changed from non-existent pyicloud_ipd.two_factor.data_types to pyicloud_ipd.sms - Fixes mypy --strict import-not-found error - All strict mypy checks now pass on both source and tests 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
… ADT method - Updated request_2sa() to use get_trusted_devices() and handle ADT result - Removed unused trusted_devices property from PyiCloudService - Removed unused PyiCloud2SARequiredException import - All tests pass, formatting and strict mypy checks clean 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…ic ADT types - Created specific ADT types for each authentication failure case: - AuthPasswordNotProvided for missing password - AuthInvalidCredentials for invalid credentials (401/421 errors) - AuthServiceNotActivated for service not activated - AuthServiceUnavailable for 503 errors - AuthAPIError for generic API errors - AuthUnexpectedError for unexpected errors - Updated authenticate_adt() to return specific ADTs instead of wrapped exceptions - Updated create_pyicloud_service_adt() to handle all new ADT types - Updated authenticator() and core_single_run() to pattern match on new ADTs - Fixed test to expect AuthInvalidCredentials instead of wrapped exception - All tests pass, strict mypy clean on code and tests This avoids the anti-pattern of wrapping exceptions in ADTs and provides more precise error types for better error handling. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Changed core_single_run return type to int | CoreSingleRunErrorResult - Added CoreSingleRunErrorResult union type to response_types.py - Updated caller with pattern matching to handle both int and ADT results - Maintained dual mode - still throws exceptions for backward compatibility - Infrastructure ready for gradual exception-to-ADT conversion - All tests pass including critical test_failed_auth_503_watch
…p 1) - Replaced sys.exit(1) with ADT return for AuthenticatorTwoSAExit - Tests still pass including critical test_failed_auth_503_watch - First successful exception-to-ADT conversion
- Changed line 1009 from raising PyiCloudConnectionException to returning ADT - Removed unused PyiCloudConnectionException import - All tests passing (218 passed, 2 skipped)
- Changed line 1010 from raising PyiCloudFailedLoginException to returning ADT - All tests passing (19 passed, 1 skipped)
- Changed line 1012 from raising PyiCloudServiceNotActivatedException to returning ADT - All tests passing (19 passed, 1 skipped)
- Changed line 1016 from raising PyiCloudAPIResponseException to returning ADT - Skipped AuthServiceUnavailable as it breaks watch functionality - All tests passing (19 passed, 1 skipped)
- Replace PyiCloudServiceNotActivatedException with ADT return - Add special logging in pattern matching to preserve error message - All tests passing (218 passed)
- Document successful conversion of 7/10 authentication errors - Identify 3 cases that must remain as exceptions - Add learnings about watch mode and MFA requirements - Note that service access errors need separate refactoring
- Replace AuthUnexpectedError with specific AuthConnectionError ADT - Only catch PyiCloudConnectionErrorException (network errors) with ADT - Let all other unexpected exceptions propagate and crash the program - This aligns with the principle that we only handle expected IO errors - Removes unnecessary complexity from error handling - All 218 tests passing
- Authentication now returns ADT results instead of throwing exceptions - Moved authentication code outside the try block - Removed PyiCloudFailedLoginException handler (no longer raised) - Converted PyiCloudFailedMFAException to direct ADT handling - Kept necessary exception handlers for service/IO errors - This completes the authentication exception-to-ADT refactoring - All 218 tests passing
…e_run - Replace PyiCloud2SARequiredException with Response2SARequired return in private libraries check - Replace PyiCloudServiceNotActivatedException with ResponseServiceNotActivated return in private libraries check - All tests pass with these minimal changes
…es check - Replace PyiCloudAPIResponseException with ResponseAPIError return - Replace PyiCloudServiceUnavailableException with ResponseServiceUnavailable return - All tests continue to pass
- Replace all four exception types with corresponding ADT returns - Maintains consistent error handling pattern - All tests pass
…h ADT return at line 1331
- Removed get_album_count function that raised exceptions - Replaced functional programming approach with direct for loop - Added inline error handling for session errors - Invalid global session errors now trigger re-authentication - Other API errors are logged and return error code directly - All tests pass, mypy strict mode passes
- Added error_occurred flag to track when retry is needed - Replaced exception raising in photo iteration with direct handling - Session errors trigger retry via flag instead of exception - WebUI errors that can be retried also use flag mechanism - Other errors return error code directly - All tests pass, mypy strict mode passes
- Replaced exception raising in download result handling with direct error handling - Used needs_retry flag to coordinate retry logic across nested loops - Session errors during download now trigger retry via flag mechanism - 500 errors and service unavailable return error code directly - Other API errors are logged but continue processing - Fixed retry logic to properly break from albums loop - All tests pass, mypy strict mode passes
- Replaced exception raising in delete photo result handling with direct error handling - Session errors during delete now trigger retry via needs_retry flag - 500 errors return error code 1 (important for delete operations) - Service unavailable returns error code 1 - Other API errors are logged but continue processing - All tests pass, mypy strict mode passes
- Replaced exception raising in autodelete result handling with direct error handling - Session errors trigger retry by continuing the while loop - WebUI errors that can be retried also continue the loop - Service unavailable returns error code 1 - Other errors return appropriate ADT results or error codes - Removed unused PyiCloud2SARequiredException import - All tests pass, mypy strict mode passes
Replace try-except block with direct ADT error handling throughout core_single_run function. All error cases now use pattern matching directly without exception catching. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
AndreyNikiforov
merged commit f09784b
into
icloud-photos-downloader:master
398 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.