Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exceptions raised in a Directive cause parallel builds to hang #8952

Closed
Labels
Milestone

Comments

@Cadair
Copy link

Cadair commented Mar 3, 2021

Describe the bug

While it seems that raising exceptions in directives is at least frowned upon, the behaviour of when this happens during a parallel build seems to not be what I would expect.

If a (custom) directive raises an exception (regular Python exception, or sphinx ExtensionError) the parallel build will hang indefinitely after printing the error to screen.

To Reproduce

Modify an extension to raise an error inside the run() method of a Directive class, then build the project with -j auto.

Expected behaviour
The build process exits with a failing code after the error is displayed.

Environment info

@Cadair Cadair added the type:bug label Mar 3, 2021
@tk0miya
Copy link
Member

tk0miya commented Mar 3, 2021

Is it really? I suppose Sphinx handles the error on the parallel build. As a trial, I added the following directive into conf.py:

from sphinx.util.docutils import SphinxDirective

class MyDirective(SphinxDirective):
    def run(self):
        raise

def setup(app):
    app.add_directive('my', MyDirective)

And sphinx-build exits successfully with displaying an error.

$ make clean html SPHINXOPTS=-jauto
Removing everything under '_build'...
Running Sphinx v3.5.1
making output directory... done
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 14 source files that are out of date
updating environment: [new config] 14 added, 0 changed, 0 removed
reading sources... [100%] subdir/foo .. subdir/qux
waiting for workers...

Sphinx parallel build error:
RuntimeError: No active exception to reraise
make: *** [Makefile:20: html] Error 2

Is there any example project?

@Cadair
Copy link
Author

Cadair commented Mar 3, 2021

I just made this example branch: https://github.com/Cadair/sunpy/tree/test_sphinx_error

I too couldn't recreate it with a very small project, maybe because there isn't enough parallel work to do to spin up enough workers or something? I have also noticed the original bug with the changelog extension on multiple projects.

tk0miya added a commit to tk0miya/sphinx that referenced this issue Mar 4, 2021
…builds to hang

Do shutdown explicitly when failure on the subprocess.
@tk0miya tk0miya added this to the 3.5.2 milestone Mar 4, 2021
@tk0miya
Copy link
Member

tk0miya commented Mar 4, 2021

Thanks. I reproduce the error now!

FROM python:3.8-slim

RUN apt update;  apt install -y build-essential curl git make unzip vim
RUN git clone https://github.com/Cadair/sunpy
WORKDIR /sunpy
RUN git checkout test_sphinx_error
RUN pip install extension-helpers
RUN pip install numpy
RUN pip install .[docs]
WORKDIR /sunpy/docs
RUN pip install matplotlib
RUN pip install sqlalchemy
RUN pip install scikit-image
RUN pip install lxml
RUN pip install zeep
RUN pip install pandas
RUN pip install h5netcdf
RUN pip install drms
RUN make html SPHINXOPTS=-jauto

And I believe #8957 will fix this.

@Cadair
Copy link
Author

Cadair commented Mar 4, 2021

Thanks a lot!

tk0miya added a commit that referenced this issue Mar 6, 2021
Fix #8952: Exceptions raised in a Directive cause parallel builds to hang
@tk0miya tk0miya closed this as completed in ec6ab72 Mar 6, 2021
This was referenced Mar 12, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.