Python bindings: proposal for enhanced exception translation

[AllSeeingEyeTolledEweSew]

Current state

Currently the python bindings translate all exceptions to RuntimeError.

The only way to apply special handling to error conditions is to check the exception message. This makes code awkward, verbose and error-prone.

In particular: in concurrent usage of libtorrent, the invalid torrent handle used error can happen on most lines of code, so special error handling is essential.

Desired state

The python bindings should raise exceptions from a meaningful type hierarchy.

Proposal

I propose the following exception type hierarchy:

Error  # subclass of RuntimeError
 +-- LibtorrentError
 |    +-- DuplicateTorrentError
 |    +-- InvalidTorrentHandleError
 +-- BDecodeError
 +-- UPNPError
 +-- SOCKSError
 +-- I2PError
_OSError  # subclass of builtin OSError and RuntimeError
 +-- _BlockingIOError    # subclass of builtin BlockingIOError
 +-- _ChildProcessError  # subclass of builtin ChildProcessError
 +-- _ConnectionError    # subclass of builtin ConnectionError
 |    +-- _BrokenPipeError         # subclass of builtin BrokenPipeError
 |    +-- _ConnectionAbortedError  # subclass of builtin ConnectionAbortedError
 |    +-- _ConnectionRefusedError  # subclass of builtin ConnectionRefusedError
 |    +-- _ConnectionResetError    # subclass of builtin ConnectionResetError
 +-- _FileExistsError      # subclass of builtin FileExistsError
 +-- _FileNotFoundError    # subclass of builtin FileNotFoundError
 +-- _InterruptedError     # subclass of builtin InterruptedError
 +-- _IsADirectoryError    # subclass of builtin IsADirectoryError
 +-- _NotADirectoryError   # subclass of builtin NotADirectoryError
 +-- _PermissionError      # subclass of builtin PermissionError
 +-- _ProcessLookupError   # subclass of builtin ProcessLookupError
 +-- _TimeoutError         # subclass of builtin TimeoutError
 +-- CanceledError        # no matching builtin exception

In a future version, we should remove RuntimeError from the exception ancestry.

The _-.prefixed _OSError tree is intended as a deprecated bridge to RuntimeError.

The exceptions should have the following features:

• Each exception type should correspond to errors of a particular error_category.
• Error and its subclasses should have a value attribute with the error value.
• OSError should correspond to both generic_category and system_category.
  • For system_category on Windows, the winerror value should match the error value
  • For system_category on non-Windows, the errno should match the error value
  • For generic_category on all platforms, the errno should match the error value

Also the following function:

"""
Returns an Exception for the given error_code, with the same logic
used when internally translating a system_error.

The intent is to be able to turn an error_code attribute of an alert into
an exception, so users can unify their error handling.
"""
def exception_from_error_code(ec: error_code) -> Exception: ...

Additionally:

• The error code values of each category should be mapped in python as enums (except for generic_category and system_category).

Rationale

I considered the following principles:

1. All exceptions must descend from RuntimeError in order not to break current usage
2. OSError corresponds closely to the intended usage of both generic_category and system_category; that is, for both generic non-system-specific errors and as a cross-platform container for system errors
3. Users should be able to catch exceptions in ways that are natural in python. That is, libtorrent.torrent_info("does-not-exist") should raise the builtin FileNotFoundError.
4. In my unscientific subjective sampling, the most common errors that need special handling are
   1. libtorrent_category with invalid_torrent_handle
   2. generic_category with ECANCELED
   3. libtorrent_category with duplicate_torrent
5. There is a semi-common practice in python to have a common ancestor for all application errors generated by a module (e.g. configparser)

In order to adhere to 1. and 3., we must duplicate the hierarchy of the builtin OSError, with copies of each subclass that descend from both their counterpart and RuntimeError. This hierarchy hasn't changed since version 3.3, so I expect this duplication to be a one-time cost.

In the future when we remove RuntimeError from the ancestry, we can remove the _OSError hierarchy. Our custom CanceledError can remain. (If CanceledError ever becomes a built-in type, our CanceledError can become a deprecated alias to that).

Our goal is to deprecate catching exceptions with except RuntimeError:. However, exception cannot tell how they are caught, so I can't think of a place where we could fire DeprecationWarning when a user writes except RuntimeError:, versus except FileNotFoundError:. I think the only thing we can do is document that RuntimeError is deprecated, and remove it in a major release.

Note that I propose a python naming scheme rather than a C++ scheme, as the exception types don't match a class in C++.

Evaluation

I have been evaluating this scheme in tvaf by translating exceptions in python, and I like it a lot. See my ltpy.py

(Since RuntimeError only has the error message and not the original error_code, I iterate error messages for all values of all known categories, and map unique error messages to their known (category, value) tuple, and then map that to the exception hierarchy as described above. Fortunately almost all error messages are unique, so this works well)

I find it especially invaluable to be able to catch InvalidTorrentHandleError.

I have actually been using a scheme in which libtorrent.OSError is a subclass of libtorrent.Error, which gives more weight to the all-exceptions-under-one-ancestor principle. But I have not found any value in doing this, and I have not found any python projects which attempt to map system errors this way. So I proposed a simpler scheme above.

[AllSeeingEyeTolledEweSew]

I started trying to do this as a PR, but I don't understand boost.python very well.

I know I need to do something like

struct error{};
struct os_error : error{};
class_<os_error, bases<error, BUILTIN_RUNTIME_ERROR, BUILTIN_OSERROR> >(...);

What should those macros be to make this work?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[AllSeeingEyeTolledEweSew]

Unstale

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[AllSeeingEyeTolledEweSew]

unstale

I found that pybind11 has some robust support for exception translation: https://pybind11.readthedocs.io/en/stable/advanced/exceptions.html

I plan to explore pybind11 more fully when I get a chance.