Add _safe_log

[drew2a]

This PR fixes #7406 i by introducing the _safe_log function.

[kozlovsky]

The PR solves the problem by introducing a new method of the Download class that should be used instead of standard logging methods. It catches possible exceptions that arise during the log message formatting.

Although the proposed change solves the problem, the chosen solution has some drawbacks:

• The original essence of the problem has nothing to do with logging. It is related to the fact that converting an alert to a string can lead to an error. At the same time, the proposed solution is explicitly tied to logging, replacing standard logging methods with a new non-standard method.
• The proposed new logging method assumes a rigid message structure when the only possible format is given inside a helper function. It looks inflexible and not versatile enough.

I would suggest considering alternative solutions:

Instead of the _safe_log method of the Download class, we could use the more generic safe_repr function, applying it to alert objects. With this approach, the developers:

• will be able to continue to use the usual methods for logging
• will not be limited in the structure of the message used when logging
• will be able to apply safe_repr to alert objects in other situations not related to logging.
• the meaning of the code will look clearer for readers since the reader will see that the original problem is not in logging as such but in the string display of the alert.
• The safe_repr function can be made generic and applicable not only to alerts but also to other types of objects - by catching all exceptions thrown when calling repr(obj), it can output some safe alternative result, for example, including the object type and its id.

[drew2a]

I believe the solution proposed in the PR is quite effective. It not only solves the problem but is also simple and straightforward. The format with f'{message}: {alert}' is sufficiently flexible to cover all calls in the nearly 1,000-line file.

The purpose of _safe_log should be clear to the reader, as it includes a comment with an explanation:

    def _safe_log(self, level: int, message: str, alert: lt.tracker_error_alert, **kwargs) -> bool:
        """ Log a message with a tracker error alert. In case of a conversion error
        from alert to string, only the message will be logged.

        Returns: True if the alert was logged, False if only the message was logged.
        """
        try:
            self._logger.log(level, f'{message}: {alert}', **kwargs)
            return True
        except Exception:  # pylint: disable=broad-except
            self._logger.log(level, message, **kwargs)
            return False

[xoriole]

In my opinion, it is possible that the alert tracker_error_alert is only raising the exception.
Here is an old ticket (also fixed): arvidn/libtorrent#143
Also, the error is reported for Windows only. So, it would be great if the issue could be actually reproduced.

PS. The link on the issue ticket is wrong. It should be #7406 instead of #7404

[drew2a]

@xoriole, thank you. I've updated the ticket in the description.

While your opinion is clear, the outcome of the review is not. What do you suggest we do with this PR?

[xoriole]

I'd agree with @kozlovsky's idea of safe_repr applying to alert objects instead of _safe_log.