diff --git a/src/ytdl_sub/cli/entrypoint.py b/src/ytdl_sub/cli/entrypoint.py index 8c6b3aab..ae89cf16 100644 --- a/src/ytdl_sub/cli/entrypoint.py +++ b/src/ytdl_sub/cli/entrypoint.py @@ -57,7 +57,7 @@ def _maybe_write_subscription_log_file( persist_log_path = Path(config.config_options.persist_logs.logs_directory) / log_filename if not success: - Logger.log_exit_exception(exception=exception, log_filepath=persist_log_path) + Logger.log_exception(exception=exception, log_filepath=persist_log_path) os.makedirs(os.path.dirname(persist_log_path), exist_ok=True) FileHandler.copy(Logger.debug_log_filename(), persist_log_path) @@ -90,7 +90,6 @@ def _download_subscriptions_from_yaml_files( Any exception during download """ subscriptions: List[Subscription] = [] - has_exception = False # Load all the subscriptions first to perform all validation before downloading for path in subscription_paths: @@ -110,7 +109,6 @@ def _download_subscriptions_from_yaml_files( else: subscription.download(dry_run=dry_run) - has_exception = has_exception or subscription.exception is not None _maybe_write_subscription_log_file( config=config, subscription=subscription, @@ -118,9 +116,7 @@ def _download_subscriptions_from_yaml_files( exception=subscription.exception, ) - if not has_exception: - Logger.cleanup() # Cleanup logger if no exceptions occurred - + Logger.cleanup(cleanup_error_log=False) gc.collect() # Garbage collect after each subscription download return subscriptions diff --git a/src/ytdl_sub/cli/output_summary.py b/src/ytdl_sub/cli/output_summary.py index e50701ca..ea133bbb 100644 --- a/src/ytdl_sub/cli/output_summary.py +++ b/src/ytdl_sub/cli/output_summary.py @@ -49,7 +49,8 @@ def output_summary(subscriptions: List[Subscription]) -> None: summary: List[str] = [] # Initialize totals to 0 - total_subs: int = 0 + total_subs: int = len(subscriptions) + total_subs_str = f"Total: {total_subs}" total_added: int = 0 total_modified: int = 0 total_removed: int = 0 @@ -57,7 +58,7 @@ def output_summary(subscriptions: List[Subscription]) -> None: total_errors: int = 0 # Initialize widths to 0 - width_sub_name: int = 0 + width_sub_name: int = len(total_subs_str) width_num_entries_added: int = 0 width_num_entries_modified: int = 0 width_num_entries_removed: int = 0 @@ -99,21 +100,18 @@ def output_summary(subscriptions: List[Subscription]) -> None: ) # Add total - total_subs += 1 total_added += subscription.num_entries_added total_modified += subscription.num_entries_modified total_removed -= subscription.num_entries_removed total_entries += subscription.num_entries total_errors += int(subscription.exception is not None) - total_subs_str = f"Total: {total_subs} Subscriptions" total_errors_str = ( - _green("All Successful") + _green("success!") if total_errors == 0 - else _red(f"{total_errors} Error{'s' if total_errors > 1 else ''}") + else _red(f"{total_errors} error{'s' if total_errors > 1 else ''}") ) - summary.append("") # new line summary.append( f"{total_subs_str:<{width_sub_name}} " f"{_color_int(total_added):>{width_num_entries_added}} " @@ -125,7 +123,7 @@ def output_summary(subscriptions: List[Subscription]) -> None: if total_errors > 0: summary.append("") - summary.append(f"See `{Logger.debug_log_filename()}` for details on errors.") + summary.append(f"See `{Logger.error_log_filename()}` for details on errors.") summary.append("Consider making a GitHub issue including the uploaded log file.") # Hack to always show download summary, even if logs are set to quiet diff --git a/src/ytdl_sub/main.py b/src/ytdl_sub/main.py index d522049a..9b7dbff3 100644 --- a/src/ytdl_sub/main.py +++ b/src/ytdl_sub/main.py @@ -24,9 +24,9 @@ def main(): """ try: _main() - Logger.cleanup() # Ran successfully, so we can delete the debug file + Logger.cleanup(cleanup_error_log=True) # Ran successfully, so we can delete the debug file except Exception as exc: # pylint: disable=broad-except - Logger.log_exit_exception(exception=exc) + Logger.log_exception(exception=exc) sys.exit(1) sys.exit(0) diff --git a/src/ytdl_sub/utils/logger.py b/src/ytdl_sub/utils/logger.py index 70bde484..263cd123 100644 --- a/src/ytdl_sub/utils/logger.py +++ b/src/ytdl_sub/utils/logger.py @@ -91,11 +91,9 @@ class Logger: # Ignore 'using with' warning since this will be cleaned up later # pylint: disable=R1732 _DEBUG_LOGGER_FILE = tempfile.NamedTemporaryFile(prefix="ytdl-sub.", delete=False) + _ERROR_LOG_FILE = tempfile.NamedTemporaryFile(prefix="ytdl-sub.errors", delete=False) # pylint: enable=R1732 - # Whether the final exception lines were added to the debug log - _LOGGED_EXIT_EXCEPTION: bool = False - # Keep track of all Loggers created _LOGGERS: List[logging.Logger] = [] @@ -108,6 +106,10 @@ class Logger: """ return cls._DEBUG_LOGGER_FILE.name + @classmethod + def error_log_filename(cls) -> str: + return cls._ERROR_LOG_FILE.name + @classmethod def set_log_level(cls, log_level_name: str): """ @@ -204,7 +206,7 @@ class Logger: redirect_stream.flush() @classmethod - def log_exit_exception(cls, exception: Exception, log_filepath: Optional[Path] = None): + def log_exception(cls, exception: Exception, log_filepath: Optional[Path] = None): """ Performs the final log before exiting from an error @@ -215,34 +217,37 @@ class Logger: log_filepath Optional. The filepath to the debug logs """ - if not cls._LOGGED_EXIT_EXCEPTION: - logger = cls.get() + logger = cls.get() - # Log validation exceptions as-is - if isinstance(exception, ValidationException): - logger.error(str(exception)) - # Log permission errors explicitly - elif isinstance(exception, PermissionError): - logger.error( - "A permission error occurred:\n%s\n" - "The user running ytdl-sub must have permission to this file/directory.", - str(exception), - ) - # For other uncaught errors, log as bug: - else: - logger.exception("An uncaught error occurred:") - logger.error( - "Version %s\nPlease upload the error log file '%s' and make a Github " - "issue at https://github.com/jmbannon/ytdl-sub/issues with your config and " - "command/subscription yaml file to reproduce. Thanks for trying ytdl-sub!", - __local_version__, - log_filepath if log_filepath else Logger.debug_log_filename(), - ) + # Log validation exceptions as-is + if isinstance(exception, ValidationException): + logger.error(str(exception)) + # Log permission errors explicitly + elif isinstance(exception, PermissionError): + logger.error( + "A permission error occurred:\n%s\n" + "The user running ytdl-sub must have permission to this file/directory.", + str(exception), + ) + # For other uncaught errors, log as bug: + else: + logger.exception("An uncaught error occurred:") + logger.error( + "Version %s\nPlease upload the error log file '%s' and make a Github " + "issue at https://github.com/jmbannon/ytdl-sub/issues with your config and " + "command/subscription yaml file to reproduce. Thanks for trying ytdl-sub!", + __local_version__, + log_filepath if log_filepath else Logger.error_log_filename(), + ) - cls._LOGGED_EXIT_EXCEPTION = True + # Any time an exception occurs, dump all debug logs into the error log + with open(cls.debug_log_filename(), mode="r", encoding="utf-8") as debug_logs, open( + cls.error_log_filename(), mode="a", encoding="utf-8" + ) as error_logs: + error_logs.writelines(debug_logs.readlines()) @classmethod - def cleanup(cls): + def cleanup(cls, cleanup_error_log: bool = False): """ Cleans up debug log file left behind """ @@ -251,6 +256,8 @@ class Logger: handler.close() cls._DEBUG_LOGGER_FILE.close() - FileHandler.delete(cls.debug_log_filename()) - cls._LOGGED_EXIT_EXCEPTION = False + + if cleanup_error_log: + cls._ERROR_LOG_FILE.close() + FileHandler.delete(cls.error_log_filename()) diff --git a/tests/unit/cli/test_entrypoint.py b/tests/unit/cli/test_entrypoint.py index 707eea10..8a298f3b 100644 --- a/tests/unit/cli/test_entrypoint.py +++ b/tests/unit/cli/test_entrypoint.py @@ -142,17 +142,17 @@ def test_subscription_logs_write_to_file( if dry_run or (mock_success_output and not keep_successful_logs): assert len(log_directory_files) == 0 return - # If not success, expect 1 log file + # If not success, expect 2 log files for both sub errors elif not mock_success_output: - assert len(log_directory_files) == 1 - log_path = log_directory_files[0] - assert bool(re.match(r"\d{4}-\d{2}-\d{2}-\d{6}\.john_smith\.error\.log", log_path.name)) - with open(log_path, "r", encoding="utf-8") as log_file: - assert log_file.readlines()[-1] == ( - f"Please upload the error log file '{str(log_path)}' and make a Github issue " - f"at https://github.com/jmbannon/ytdl-sub/issues with your config and " - f"command/subscription yaml file to reproduce. Thanks for trying ytdl-sub!\n" - ) + assert len(log_directory_files) == 2 + for log_path in log_directory_files: + assert bool(re.match(r"\d{4}-\d{2}-\d{2}-\d{6}\.john_smith\.error\.log", log_path.name)) + with open(log_path, "r", encoding="utf-8") as log_file: + assert log_file.readlines()[-1] == ( + f"Please upload the error log file '{str(log_path)}' and make a Github issue " + f"at https://github.com/jmbannon/ytdl-sub/issues with your config and " + f"command/subscription yaml file to reproduce. Thanks for trying ytdl-sub!\n" + ) # If success and success logging, expect 3 log files else: assert len(log_directory_files) == num_subscriptions