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

ExtensionError shows no information about what extension caused it #8813

Closed
mgeier opened this issue Feb 2, 2021 · 9 comments
Closed

ExtensionError shows no information about what extension caused it #8813

mgeier opened this issue Feb 2, 2021 · 9 comments
Labels
extensions type:proposal a feature suggestion
Milestone

Comments

@mgeier
Copy link
Contributor

mgeier commented Feb 2, 2021

Currently, when an ExtensionError happens during build, the text Extension error: followed by the actual error message is shown.

This is helpful if the error message from the extension is clear about what to do to fix the problem.

However, if there are many extensions and the error message isn't quite as clear, a user might have no idea where this message even came from.

Of course, extension authors could try to include their extension name in each of the possible error messages, but I don't think this will happen in many cases and even if the extension name is added in some cases, it's easy to miss it in some other cases.

Would it be possible/feasible for Sphinx to check which extension caused which ExtensionError and include the extension name in the error message?

Describe the bug

An ExtensionError doesn't show which extension caused it.

To Reproduce

conf.py:

extensions = ['sphinx.ext.intersphinx']
intersphinx_mapping = {
    'sphinx': NotImplemented,
}

This produces the following error message:

Extension error:
Handler <function load_mappings at 0x7f5e4d217310> for event 'builder-inited' threw an exception (exception: argument of type 'NotImplementedType' is not iterable)

Expected behavior

Extension error (sphinx.ext.intersphinx):
Handler <function load_mappings at 0x7f5e4d217310> for event 'builder-inited' threw an exception (exception: argument of type 'NotImplementedType' is not iterable)

This doesn't make the error message itself better, but at least I know I should start looking at the intersphinx extension and its settings ...

@mgeier mgeier added the type:bug label Feb 2, 2021
@tk0miya tk0miya added extensions type:proposal a feature suggestion and removed type:bug labels Feb 2, 2021
@tk0miya tk0miya added this to the 3.5.0 milestone Feb 2, 2021
@tk0miya
Copy link
Member

tk0miya commented Feb 2, 2021

Good point!

tk0miya added a commit to tk0miya/sphinx that referenced this issue Feb 2, 2021
…nt handler

Show the module name of the event handler on the error inside it to let
users know a hint of the error.
tk0miya added a commit to tk0miya/sphinx that referenced this issue Feb 4, 2021
…nt handler

Show the module name of the event handler on the error inside it to let
users know a hint of the error.
tk0miya added a commit to tk0miya/sphinx that referenced this issue Feb 4, 2021
…nt handler

Show the module name of the event handler on the error inside it to let
users know a hint of the error.
@tk0miya tk0miya closed this as completed in 654458b Feb 5, 2021
tk0miya added a commit that referenced this issue Feb 5, 2021
Close #8813: Show what extension caused it on errors on event handler
@mgeier
Copy link
Contributor Author

mgeier commented Feb 5, 2021

Thanks for #8822, which fixes my example situation!

However, I was hoping for a more general solution ... let me try to come up with another, more realistic example:

my_extension.py, somewhere in Python's path:

import sphinx

def helper_function():
    raise NotImplementedError('This is a dummy function!')

def _env_updated(app, env):
    try:
        helper_function()
    except Exception as e:
        raise sphinx.errors.ExtensionError('An error happened', e) from e

def setup(app):
    app.connect('env-updated', _env_updated)

conf.py:

extensions = [
    'my_extension',
]

This produces the following error message:

Extension error:
An error happened (exception: This is a dummy function!)

... while I think it would be more helpful to get something like this:

Extension error (my_extension):
An error happened (exception: This is a dummy function!)

@tk0miya
Copy link
Member

tk0miya commented Feb 6, 2021

Oops... is there any extension using ExtensionError directly? It will go well if it uses another exception instead.

@mgeier
Copy link
Contributor Author

mgeier commented Feb 7, 2021

is there any extension using ExtensionError directly?

Yes, I'm using ExtensionError in my extensions, because it was the easiest way I could find to stop the build process and print a friendly error without intimidating the user with a traceback.

Is there another, better way to achieve this?

Raising any other exception created a traceback when I last tried.

@tk0miya tk0miya reopened this Feb 7, 2021
@tk0miya
Copy link
Member

tk0miya commented Feb 7, 2021

I think we've not provided an official way to do that. I agree ExtensionError is appropriate for such a purpose. How about passing modname argument for the extension?

@mgeier
Copy link
Contributor Author

mgeier commented Feb 7, 2021

I've had a quick look at the Sphinx history and I found that there was a change (which I wasn't aware of) with release 3.0.1, more specifically in #7658.

This is great, because now any exception (except if derived from SphinxError) thrown by an extension is automatically turned into an ExtensionError.

This means, starting with Sphinx 3.0.1 it is not necessary anymore for extension authors to explicitly use ExtensionError.

I think we've not provided an official way to do that. I agree ExtensionError is appropriate for such a purpose. How about passing modname argument for the extension?

Since the change I mentioned above, there is no reason to recommend using ExtensionError anymore.
In fact, extension authors don't have to worry about this anymore at all, which is great!

I have only used ExtensionError to work around the problem of showing ugly tracebacks to the user. But luckily, this work-around isn't necessary anymore when using Sphinx>=3.0.1.

The question is now: what should extensions do that already use ExtensionError?

I'm not using this in my own extensions anymore (I'm currently removing the last instance in mgeier/sphinx-last-updated-by-git#25), but I've found one instance there:

https://github.com/executablebooks/rst-to-myst/blob/6b38f8691d8bac1e6168d11b4c27b6983fae90f0/rst_to_myst/namespace.py#L231-L235

I guess the easiest solution would be to simply wait until the extension doesn't support Sphinx<3.0.1 anymore, and then get rid of any uses of ExtensionError.

But if Sphinx can somehow handle these cases automatically, this would be great.

@tk0miya
Copy link
Member

tk0miya commented Feb 7, 2021

The question is now: what should extensions do that already use ExtensionError?

I don't know what are you asking. AFAIK, there are no recommendation to use ExtensionError for 3rd party developers. So I've never known there are extensions using it ever. And, there is no side-effect if they're using it. Indeed, module name will not be shown. But the behavior is the same as ever. As in the past, they can use ExtensionError and nothing changed.

@tk0miya tk0miya modified the milestones: 3.5.0, 4.0.0 Feb 14, 2021
@mgeier
Copy link
Contributor Author

mgeier commented Feb 16, 2021

Yes ExtensionError behaves as it did before.
I just thought it might also automatically show the name of the extension that caused it, but this isn't really important.

@tk0miya
Copy link
Member

tk0miya commented Mar 2, 2021

I think there is nothing to do for this topic. So I'm closing this now.
Thanks,

@tk0miya tk0miya closed this as completed Mar 2, 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.
Labels
extensions type:proposal a feature suggestion
Projects
None yet
Development

No branches or pull requests

2 participants