don’t intercept exceptions in AtomicShortcode and BlockShortcode

[kmohrf]

hi,

first and foremost: thank you for working on this library.

while looking at the code (and integrating it into one of my django apps) i noticed that you except any kind of exception in AtomicShortcode and BlockShortcode with no possibility to opt-out from this behaviour.

imho you should’t catch exceptions at all at this point. in fact the way you handle them now even hides SyntaxError exceptions. in my case i wanted to raise a specific exception that is handled by one of the django middlewares. this is impossible because the exception in my shortcode is already handled by the render function in AtomicShortcode. all i have now is a RenderingError that doesn’t help much :).

i see a few options for resolving this:

• don’t catch exceptions in AtomicShortcode and BlockShortcode
• allow users to configure the behaviour when registering the shortcode
• expose a custom exception class that users can subclass and that is ignored by your exception handler function

if you tell me which option you prefer i may be able to submit a pull request :).

[dmulholl]

Hi Konrad. Thanks for filing such a detailed and helpful report.

The rationale for wrapping rendering-callback exceptions is so we can guarantee that the library will only ever throw a ShortcodeError exception (or one of its subclasses). This is a useful feature if the code is going to be running unattended on a server.

You're absolutely right that it's important to preserve the original exception, but Python 3 actually does this automatically. If you raise a new exception from within an except block then the original exception is preserved as the new exception's __context__ attribute.

This is pretty obscure behaviour so I'm going to add notes to the code and documentation to make it clear that it's happening.

Also, looking again at the docs and the original PEP, I think we should probably be using the explicit __cause__ attribute as well as relying on __context__ being set automatically.

I can add this without compromising backwards compatibility so I'll push an update tonight or tomorrow.

I think looking at __context__ or __cause__ should solve your problem but I'm not familiar with how Django middleware operates so let me know if there's still an issue.

[kmohrf]

i guess you’re suggesting to use the new raise … from … syntax? but it’s good to know that raise sets the context of the new exception to the one that was catched. i didn’t know that.

as far as i’m concerned that’s everything i need as i have control over the middleware and can just catch the RenderingError and use the __context__, though there might still be a use case for configuring the behaviour.

thank you for taking the time!

[dmulholl]

I just pushed version 2.4.0 which uses the raise ... from ... syntax to explicitly wrap handler exceptions. Thanks for bringing up the issue - I think it's much clearer now what's actually happening in the code.