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

Prevent plugins that cannot be instantiated from being loaded #3956

Merged
merged 4 commits into from Dec 4, 2018
Merged

Prevent plugins that cannot be instantiated from being loaded #3956

merged 4 commits into from Dec 4, 2018

Conversation

bennothommo
Copy link
Contributor

Per @LukeTowers suggestion here, this functionality wraps a portion of the loadPlugin function with a try..catch block. If, for any reason, the plugin cannot be instantiated (for example, if a parse error occurs or reserved keywords are used in the namespace), instead of bricking the site, this will simply prevent the plugin from being loaded.

@LukeTowers
Copy link
Contributor

This is great @bennothommo! Could we either throw an exception or log an error when an invalid plugin is detected instead of just chugging along as if nothing happened though? What are your thoughts on that?

@bennothommo
Copy link
Contributor Author

@LukeTowers Good idea. I did try having some logging originally, however, when it was using a translatable string it caused all the system translation strings to remain untranslated. I can only assume that when the plugins are loaded, the translator isn't fully operational at that point. It does work when using a static string however.

@LukeTowers
Copy link
Contributor

I'm fine with a static string, at least then people would be able to google the issue. And yes, that issue does happen when attempting to use the translator before everything finishes loading, I've had to wrap code that uses the translator directly in App::before() to get it working properly but I don't think that would work in this case.

@bennothommo
Copy link
Contributor Author

Error log now in place :)

@LukeTowers LukeTowers merged commit 2002fd6 into octobercms:develop Dec 4, 2018
@LukeTowers
Copy link
Contributor

Great job @bennothommo, thanks very much!

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

Successfully merging this pull request may close these issues.

None yet

2 participants