-
-
Couldn't load subscription status.
- Fork 3.2k
RFC: Change behaviour or how exceptions are silently caught in production and edit mode #7953
-
PR #7423 introduced graceful plugin rendering which is a very reasonable and helpful solution regarding the edit mode.
However, this breaks with existing DjangoCMS code bases when trying to move forward to v4.1:
-
If there are plugins raising an exception due to a bug, it's sometimes more reasonable to issue a
HTTP 500instead of simply skipping this plugin. This could have unintended effects since even if there is a monitoring to logging filter such failures, it's still aHTTP 200and can be cached, read by crawlers and search engines etc. -
It may happen that there is a child plugin raising a specific exception which can be handled by its parent. This applies to the edit mode as well.
I first wanted to open this thread as this seems like a behavioral change which might require further consideration and discussion.
Beta Was this translation helpful? Give feedback.
All reactions
-
👀 3
Replies: 5 comments 6 replies
-
Interesting. The original PR was about prevention of uneditable pages (i.e. protecting the edit mode). I guess that makes sense.
In regards to error monitoring I presume these errors must be detectable. For example it must be possible to catch all errors with sentry.io or similar error monitoring tools on staging and production environments, regardless of whether the page throws a 500 Server Error or is at least partially rendered. An example of such sentry.io configuration in the django CMS documentation might be recommendable.
In regards to SEO and UX - I have doubts that showing a 500 Server Error is better than showing an incomplete page. I guess in the best case this behaviour could be made configurable through a settings variable such as CATCH_PLUGIN_500_EXCEPTION = True
In regards to parent plugins should be able to catch child plugin exceptions - that appears not possible in all cases and configurations as the error bubbles up through the django template system and not through plugin code. (edited as per following post's info)
I therefore think that all requirements can be resolved without the need to change the original intention of the underlying PR.
Beta Was this translation helpful? Give feedback.
All reactions
-
I am not sure how parent plugins can catch errors of child plugins - unless you have a customized render_plugin template tag. In CMS3, the rendering process used to stop once an exception was raised, and it propagates up through Django's template system not touching any plugin code. @florianschieder Can you give some more info on that?
Django has two opposing conventions:
- Server error if template tags fail
- Silent failure if variable lookups fail (including running methods on objects)
I'd like to think of plugins as "variables" that are moved by the editors. But I can imagine a setting that would trigger a 500 as opposed to ignoring content which would honour the other convention.
Beta Was this translation helpful? Give feedback.
All reactions
-
I am not sure how parent plugins can catch errors of child plugins - unless you have a customized
render_plugintemplate tag. In CMS3, the rendering process used to stop once an exception was raised, and it propagates up through Django's template system not touching any plugin code. @florianschieder Can you give some more info on that?
Exactly. We have got a custom template tag for this use case called render_plugin_gracefully.
Beta Was this translation helpful? Give feedback.
All reactions
-
Interesting. What does that do?
Beta Was this translation helpful? Give feedback.
All reactions
-
In our case it almost does the same what PR #7423 introduced: it catches exceptions and renders the plugin as an empty string. But we only do this for particular exceptions, not all.
Concrete example: There is a wrapper plugin which has the following child plugins:
- product heading
- image
- custom product plugin loading product data from an external webservice
- text plugin
Our use case is that if the external data cannot be loaded (because it is not yet published within the external system) it raises a particular exception handled by the wrapper plugin. The wrapper plugin is responsible for not rendering any of the child plugins, as it would be wrong content if the heading, image and text plugin are rendered but not the custom product plugin - which would be the case if there is no chance that the exceptions caught via #7423 can be re-raised e.g. via a Django settings.py switch.
Beta Was this translation helpful? Give feedback.
All reactions
-
I think this can be easily solved by a setting. The setting imho would only affect the live pages, in preview and edit you should still see the error and the other plugins (or otherwise they would not be editable).
Beta Was this translation helpful? Give feedback.
All reactions
-
+1. Sounds like a good solution to me.
Beta Was this translation helpful? Give feedback.
All reactions
-
It sounds like the 500 error is useful to development/testing a site, whereas it's problematic to a production project. (Which is why we made the change.)
Perhaps a setting that raises the exception instead of logging the error might make sense?
Beta Was this translation helpful? Give feedback.
All reactions
-
+1. Sounds like a good solution to me as long as the edit mode still shows the error (see fsbrauns comment]
Beta Was this translation helpful? Give feedback.
All reactions
-
@macolo @marksweb @fsbraun
should I file an issue for the approach of introducing a setting which only affects live pages, but keeping the original intention of #7423 that in preview and edit mode an error section is shown and the page is still editable?
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 2
-
→ #7974
Beta Was this translation helpful? Give feedback.