Do not allow exceptions to propagate from backend

[stephenfin]

The pyproject_api project provides an implementation of a PEP-517 build frontend. Consider the architecture described in the proposal [1]:

+-----------+          +---------------+           +----------------+
| frontend  | -spawn-> | child cmdline | -Python-> |    backend     |
|           |          |   interface   |           | implementation |
+-----------+          +---------------+           +----------------+

We can map the these components to 'pyproject_api' components like so:

• frontend ->
  pyproject_api._frontend.Frontend (or rather, a concrete implementation of same)

• child cmdline implementation ->
  pyproject_api._backend

• backend implementation ->
  (the backend defined by the [build-system] build-backend key in pyproject.yaml, typically as parsed by the pyproject_api._frontend.Frontend.create_args_from_folder method)

In both the pyproject_api._via_fresh_subprocess.SubprocessFrontend and tox.tox_env.python.virtual_env.package.pyproject.Pep517VirtualEnvFrontend concrete implementations of pyproject_api._frontend.Frontend, the child cmdline implementation provided by pyproject_api._backend is called via subprocess. However, there is a crucial difference: while the former will start a new subprocess for each command invoked, the latter will reuse the backend. This can be problematic. Communication between the frontends and the child cmdline implementation is done by issuing commands in serialized JSON format to the pyproject_api._backend executable via stdin, then waiting for a special Backend: Wrote response foo to $file message to appear on stdout, before finally reading the contents of the (JSON-formatted) file at $file.

In the success case, the file at $file will contain a single key, result, while in the error case the file will contain three keys, code, exc_type and exc_msg. The contents of the latter keys are generated by catching any exception raised by the build backend and processing it. However - and here is the crux of the issue - if these exceptions are not derived from Exception then we allow them to bubble up, killing the backend process in the process. A comment inline suggests this is intended to allow things like SystemExit or KeyboardInterrupt, but setuptools' (or rather, distutils') exceptions do not derive from Exception either.

This is an issue for frontends that re-use backends like the Pep517VirtualEnvFrontend provided by tox. These backends expect to politely cleanup when the backend fails (as indicated by the presence of code, exc_type and exc_msg keys in the result file) by calling a special _exit command on the backend. However, if the backend process has died, there's nothing left to answer this request. This causes tox at least to hang forever.

There are a two solutions to this. Firstly, we can remove the special case for these two exceptions thus allowing the frontend to manage the lifecycle of the _backend executable process regardless. Alternatively, we could add a new key to the result file indicating that the backend process has died and the frontend should not expect to be able to do any cleanup. The latter option would require changing the "interface" defined by this result file and updating all frontend implementations to support this. As a result, we opt for the former as the simpler choice. A well behaved frontend should ensure a backend process is never allowed to run forever so the end-user experience should be identical.

[*] Note that this is different to errors the occur while initially starting the backend executable or processing a command. If the backend fails to start, a separate special message is issued (failed to start backend), which the frontend is expected to handle, and the exception is re-raised. Invalid requests, meanwhile, are simply ignored and a message is logged which the frontend could choose to handle.

Closes: #39

[gaborbernat]

Can you add a test?

[gaborbernat]

Can we add a test for this please, otherwise looks good.

[stephenfin]

Can we add a test for this please, otherwise looks good.

Done 👍

[gaborbernat]

You'll need to please the CI gods 😅 and add a changelog entry.

[stephenfin]

I added the missing types to (hopefully) fix the CI. I held off updating the changelog since I don't know what the version number will be.

(As an aside, it would be nice to use something like reno or towncrier here.

[gaborbernat]

1.3.0 would be a fine version.