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

sys.exit called on rst parse errors #1759

Open
valhallasw opened this Issue Jun 16, 2015 · 6 comments

Comments

Projects
None yet
6 participants
@valhallasw

valhallasw commented Jun 16, 2015

The docutils.core.Publisher backend is called in a such a way that it doesn't raise an Exception, but calls sys.exit instead. This makes it harder to use pelican as library.

How to reproduce this?

Using an rst file with invalid syntax, run:

from pelican import Pelican
from pelican.settings import read_settings

p = Pelican(read_settings('pelicanconf.py'))
try:
    p.run()
except Exception as e:
   print "Caught exception: ", e

print "Continue after failure"

Expected behavior:

$ python test.py
....../file.rst:29: (ERROR/3) Unknown directive type "fdsaf".

<details>

Caught exception: SomeException
Continue after failure

Actual behavior:

$ python test.py
....../file.rst:29: (ERROR/3) Unknown directive type "fdsaf".

<details>

$ (silently exits)

This is caused because of enable_exit_status=True which is passed to docutils.core.Publisher.publish since #1224 was merged.

Unfortunately, I don't know enough about the internal structure of Pelican to suggest where the docutils Exception should be changed into a sys.exit, but I'm inclined to say this should be close to the entry point. This allows people who call Pelican as a library to catch an Exception as-is, instead of resorting to the workaround

try:
    p.run()
except SystemExit as e:
   print "Caught sys.exit."
@danmackinlay

This comment has been minimized.

Show comment
Hide comment
@danmackinlay

danmackinlay Jun 18, 2015

Moreover, it makes hard to work out where exceptions are raised, since the original traceback is lost.

danmackinlay commented Jun 18, 2015

Moreover, it makes hard to work out where exceptions are raised, since the original traceback is lost.

@avaris

This comment has been minimized.

Show comment
Hide comment
@avaris

avaris Jun 19, 2015

Member

I'm not sure why this was considered a good idea in the first place. Exiting the process for an error in a single .rst file does sound like the definition of overkill. Instead it should log the error and skip the file (like we do for any other reader level error).

Member

avaris commented Jun 19, 2015

I'm not sure why this was considered a good idea in the first place. Exiting the process for an error in a single .rst file does sound like the definition of overkill. Instead it should log the error and skip the file (like we do for any other reader level error).

@omiday

This comment has been minimized.

Show comment
Hide comment
@omiday

omiday Oct 19, 2016

Contributor

I was going to submit a PR but first need to fix #2032.

Contributor

omiday commented Oct 19, 2016

I was going to submit a PR but first need to fix #2032.

@omiday

This comment has been minimized.

Show comment
Hide comment
@omiday

omiday Oct 21, 2016

Contributor

@justinmayer
actually #2032 deals with a different set of files and since I have the commit ready I could go ahead and file the PR for this one

Contributor

omiday commented Oct 21, 2016

@justinmayer
actually #2032 deals with a different set of files and since I have the commit ready I could go ahead and file the PR for this one

omiday added a commit to omiday/pelican that referenced this issue Nov 5, 2016

@leotrs

This comment has been minimized.

Show comment
Hide comment
@leotrs

leotrs Dec 13, 2016

@omiday did that PR ever get done?

leotrs commented Dec 13, 2016

@omiday did that PR ever get done?

omiday added a commit to omiday/pelican that referenced this issue Mar 2, 2017

@justinmayer

This comment has been minimized.

Show comment
Hide comment
@justinmayer

justinmayer Mar 23, 2018

Member

Status: PR #2047 still awaiting review.

Member

justinmayer commented Mar 23, 2018

Status: PR #2047 still awaiting review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment