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

[unitaryhack] feat: try-catch on plugin loading #545

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

anushkrishnav
Copy link

Added a try-catch block inside the plugin loading loop to handle load failure
Issues #517

@anushkrishnav anushkrishnav changed the title feat: try-catch on plugin loading [unitaryhack] feat: try-catch on plugin loading Jun 11, 2022
@amccaskey
Copy link
Contributor

Hey @anushkrishnav a few things here. First off you need to make sure that you sign your commits and that your email in the signature matches the email on your Eclipse account. You need to have an Eclipse account and make sure you go in and sign the contributor licensing agreement. Without this the PR will not pass the IP checks.

Next thing - the current code commit you have will not address the issue. The plugin loading error is happening at the Start() method invocation. It is wrapped in a try catch, we really just need that snippet to be updated to throw a warning instead of an error (which causes the framework to stop). A warning here will let the framework continue running, but will just not have the erroneous plugin available.

@amccaskey
Copy link
Contributor

Though - you might also keep a try / catch around the InstallBundles calls too. Just make sure you issue a warning instead of an error.

@anushkrishnav
Copy link
Author

Hey @anushkrishnav a few things here. First off you need to make sure that you sign your commits and that your email in the signature matches the email on your Eclipse account. You need to have an Eclipse account and make sure you go in and sign the contributor licensing agreement. Without this the PR will not pass the IP checks.

Next thing - the current code commit you have will not address the issue. The plugin loading error is happening at the Start() method invocation. It is wrapped in a try catch, we really just need that snippet to be updated to throw a warning instead of an error (which causes the framework to stop). A warning here will let the framework continue running, but will just not have the erroneous plugin available.

SO basically instead of a xacc:error it needs to use xacc::exception

@amccaskey
Copy link
Contributor

Instead of xacc::error it should be xacc::warning.

@anushkrishnav
Copy link
Author

Done

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

Successfully merging this pull request may close these issues.

2 participants