END_HANDLE_TH_ERRORS_PYBIND prevents pybind11 Exception translation

[cpuhrsch]

🐛 Bug

pybind11 allows exception translation by catching and translating C++ exceptions. This is similar to what we do within END_HANDLE_TH_ERRORS_PYBIND as well, in the sense that we also inspect the thrown type and handle it differently in some cases (e.g. py::builtin_exception). However, due to CATCH_ALL_ERRORS we prevent pybind from catching something like a std::out_of_range and instead bubble up a PyExc_RuntimeError. For the pybind11 user that's writing code out of tree and uses wrap_pybind_function as exception translation for at:: specific errors this means they have to setup their own translation within the wrapped function to get the proper error.

To Reproduce

To reproduce this behavior register a simple pybind11 function that throws out_of_range and see it turn into RuntimeError and not IndexError. I can provide a code snippet if needed.

Expected behavior

I'd expect us to simply forward the exception and rely on pybind11 to do the proper translation for the listed functions, so we can simply forward them. I think this is ok, because END_HANDLE_TH_ERROR_PYBIND is explicitly written for pybind11.

Environment

$ python torch/utils/collect_env.py
Collecting environment information...
PyTorch version: 1.5.0a0+45c4519
Is debug build: Yes
CUDA used to build PyTorch: Could not collect

OS: Ubuntu 18.04.3 LTS
GCC version: (Ubuntu 7.4.0-1ubuntu1~18.04.1) 7.4.0
CMake version: version 3.12.2

Python version: 3.7
Is CUDA available: No
CUDA runtime version: 9.2.88
GPU models and configuration:
GPU 0: Quadro GP100
GPU 1: Quadro GP100

Nvidia driver version: 418.116.00
cuDNN version: Could not collect

Versions of relevant libraries:
[pip3] gpytorch==0.2.1
[pip3] numpy==1.16.4
[pip3] torch==1.5.0a0+45c4519
[pip3] torchvision==0.6.0a0+0156d58
[conda] blas                      1.0                         mkl
[conda] gpytorch                  0.2.1                    pypi_0    pypi
[conda] magma-cuda100             2.5.0                         1    pytorch
[conda] mkl                       2019.4                      243
[conda] mkl-include               2019.4                      243
[conda] mkl-service               2.3.0            py37he904b0f_0
[conda] mkl_fft                   1.0.15           py37ha843d7b_0
[conda] mkl_random                1.1.0            py37hd6b4f25_0
[conda] torch                     1.5.0a0+45c4519           dev_0    <develop>
[conda] torchvision               0.6.0a0+0156d58           dev_0    <develop>

Compiled from source

Additional context

This showed up in the context of porting NestedTensor.to_tensor into C++.

[cpuhrsch]

@albanD - you created #30027 which is affected by this.

[albanD]

cc @peterbell10 that changed the CATCH_TH_ERROR to CATCH_ALL_ERROR in there #30588. Is that the expected behavior or a typo that we should fix?
I would agree with @cpuhrsch that in this case we don't want to catch all, but I might be missing something?

[peterbell10]

Prior to #30588, CATCH_TH_ERRORS was the only macro and it was equivalent to the current CATCH_ALL_ERRORS, including catching std::exception. The reason it does that is to translate C++ type names in the error message into the python-friendly type names (torch::processErrorMsg).

AFAIK, there is no way to catch a generic std::exception and edit the error message without loosing the type of the original exception.

[albanD]

Ho right so the reason is to translate cpp names into python names.

I guess we need a to make a choice:

• We keep general exception types but we don't have nice names except for our own exceptions.
• We always change the names but we override the exception types (current behavior).