Refactor exception handling: Define custom exceptions as subclasses of a base Errors class

castanley commented 2 months ago

Description

This pull request refactors the exception handling in protect_archiver. The following changes were made:

Defined custom exceptions ProtectError, DownloadFailed, and AuthorizationFailed as subclasses of a base Errors class in errors.py.
Updated download_file.py to correctly import these exceptions.

Changes

errors.py: Defined ProtectError, DownloadFailed, and AuthorizationFailed as subclasses of Errors.
download_file.py: Updated import statements to use the newly defined exceptions.
Updated other modules to use the new exception hierarchy.

Motivation and Context

This refactor resolves the following issues:

AttributeError: 'NoneType' object has no attribute 'get'
- This error occurred when handling certain types of responses that resulted in None being accessed incorrectly.
TypeError: catching classes that do not inherit from BaseException is not allowed
- This error was caused by improperly defined custom exceptions that did not inherit from BaseException. The new hierarchy ensures that all custom exceptions are correctly defined as subclasses of Errors, which inherits from Exception.

How Has This Been Tested?

Verified that the exceptions are defined correctly.
Ensured the download_file.py and other modules import and use the exceptions without issues.
Rebuilt the package and tested with Poetry the functionality to confirm the changes are effective.
Note: Only the download function has been tested. Other functions have not been tested.

codecov[bot] commented 4 weeks ago

Codecov Report

Attention: Patch coverage is 42.85714% with 16 lines in your changes missing coverage. Please review.

Project coverage is 53.65%. Comparing base (80e3969) to head (8a0432c). Report is 46 commits behind head on main.

Files with missing lines	Patch %	Lines
protect_archiver/downloader/download_file.py	20.00%	8 Missing :warning:
protect_archiver/cli/download.py	0.00%	2 Missing :warning:
protect_archiver/cli/events.py	0.00%	2 Missing :warning:
protect_archiver/errors.py	80.00%	2 Missing :warning:
protect_archiver/client/legacy.py	50.00%	1 Missing :warning:
protect_archiver/client/unifi_os.py	50.00%	1 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #465 +/- ## ========================================== - Coverage 56.81% 53.65% -3.16% ========================================== Files 23 24 +1 Lines 778 820 +42 ========================================== - Hits 442 440 -2 - Misses 336 380 +44 ``` | [Flag](https://app.codecov.io/gh/danielfernau/unifi-protect-video-downloader/pull/465/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniel+Fernau) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/danielfernau/unifi-protect-video-downloader/pull/465/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniel+Fernau) | `53.65% <42.85%> (-3.16%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniel+Fernau#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

danielfernau commented 4 weeks ago

Hey @castanley - thanks a lot for your contribution to the project! I've integrated your suggestions with some additional housekeeping and tweaks for optimization.

Your changes are now merged! 🚀

Looking forward to more collaboration in the future.

Best, Daniel

danielfernau / unifi-protect-video-downloader