Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Do not allow exceptions to propagate from backend
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. An alternative option would be to add a new key to the result file indicating the backend process has died and the frontend should not expect to be able to do any cleanup, but this would require changing the "interface" defined by this result file and updating all frontend implementations to support this. In this change, we opt for the former since it's simpler and a well behaved frontend should ensure a backend process is never allowed to run forever. [*] 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. Signed-off-by: Stephen Finucane <[email protected]> Closes: #39
- Loading branch information