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

Communicate errors with Exceptions where appropriate #292

Closed
dmitry-timofeev opened this issue Mar 1, 2019 · 3 comments
Closed

Communicate errors with Exceptions where appropriate #292

dmitry-timofeev opened this issue Mar 1, 2019 · 3 comments
Milestone

Comments

@dmitry-timofeev
Copy link
Contributor

Currently some operations in PluginManager may not complete successfully, according to documentation:

  • PluginManager#loadPlugin — returns null if failed to load the plugin
  • PluginManager#unloadPlugin — returns false if failed to unload the plugin

Such return values

  • Are quite easy to miss to check (and get an error down the road).
  • Do not communicate why the PluginManager failed to perform the requested operation.

The list is not exhaustive, and currently includes only the operations I bumped into. There might be some other operations that might benefit from more direct and detailed error reporting.

See also

@dmitry-timofeev dmitry-timofeev mentioned this issue Mar 1, 2019
3 tasks
@decebals decebals added this to the PF4J 3 milestone Mar 26, 2019
decebals added a commit that referenced this issue Mar 29, 2019
@decebals
Copy link
Member

I added throws PluginException in the signature of PluginManager#loadPlugin method.
I think that for PluginManager#unloadPlugin is not necessary.

From mailing list discussion:

Other discussion is what kind of exception we prefer: checked or unchecked (runtime) exception. Is it a good to create a PluginRuntimeException? For example PluginManager#loadPlugin will throw PluginException or PluginRuntimeException?

In the end I opted for a checked exception.

@dmitry-timofeev
Copy link
Contributor Author

Thank you, it makes sense — the clients that explicitly load plugins are likely to need to recover when a plugin turns un-loadable. Regarding unloadPlugin — can this operation really fail?

decebals added a commit that referenced this issue Apr 20, 2019
@decebals
Copy link
Member

Now I think this feature is fully implemented.

decebals added a commit that referenced this issue Apr 20, 2019
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

No branches or pull requests

2 participants