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

Provide diagnostic information when plugin loading fails. #51

Closed
plexus opened this issue Jan 24, 2019 · 6 comments
Closed

Provide diagnostic information when plugin loading fails. #51

plexus opened this issue Jan 24, 2019 · 6 comments
Assignees
Labels
ergonomics Making Kaocha delightful to work with and people more efficient first issue only These tickets are only for first-time contributors. improvement Incremental improvement of an existing feature

Comments

@plexus
Copy link
Member

plexus commented Jan 24, 2019

A follow-up to #35 / #42. Currently when a plugin isn't found, this is what it looks like

2019-01-24-174315_463x108_scrot

It would be good if we could give the user some kind of indication of what might be going wrong.

@plexus plexus added improvement Incremental improvement of an existing feature first issue only These tickets are only for first-time contributors. ergonomics Making Kaocha delightful to work with and people more efficient labels Jan 24, 2019
@AndreaCrotti
Copy link
Contributor

Would it not be enough to print out the list of available plugins in these cases?
That should hopefully clarify what they are doing wrong automatically.

@plexus
Copy link
Member Author

plexus commented Jun 20, 2019

Any namespace on the classpath could potentially be a plugin, so that's not really feasible.

@noorfathima11
Copy link

Can this be assigned to me?

@plexus
Copy link
Member Author

plexus commented Mar 15, 2020

That would be great @noorfathima11 , thanks for looking into this. You'll find the code in kaocha.plugin. What happens there is two things

First we try to load the plugin namespace based on the plugin name. How it does this depends on what kind of name the plugin has.

If it's a namespaced (qualified) keyword like :foo.bar/baz then we try to first load foo.bar.baz, and if that doesn't succeed then we try to load foo.bar.

If it's a simple keyword that contains a period then we load the corresponding namespace, so :foo.bar.baz -> (require 'foo.bar.baz). If there is no period in the name, e.g. :notifier, then we consider the plugin to be under kaocha.plugin, so we (require 'kaocha.plugin.notifier) (that last bit is new, I've been wanting to do that forever, code coming to master today).

This namespace loading completely ignores all errors, it really doesn't care if it finds a namespace or not, it proceeds regardless.

The namespace that we try to load is expected to contain a (defmethod kaocha.plugin/-register :the-plugin-name ...). (or a (defplugin ...), which really does the same thing). After trying to load the namespace, we call -register, and this is the point where you get the error message you see above. It's printed in the :default implementation of -register, so it's what you get if no specific -register has been defined for the plugin.

So to give better diagnostics this code is going to have to become smarter. try-load-third-party-namespaces should return information about which namespaces we tried to load, and which one of them actually loaded. The default version of -register should not throw immediately, but it should return a special value so that the caller knows that the default was called.

In register (without the -) we now get all the information we need to generate a proper error message. We can check if the default version of -register got called, and if so print out a message saying which files we attempted to load, and which ones did or did not load.

For example:

ERROR: Failed loading plugin :foo.bar.baz, no such namespace foo.bar.baz.
ERROR: Failed loading plugin :foo.bar/baz, no such namespace foo.bar or foo.bar/baz.
ERROR: Failed loading plugin :foo.bar/baz, loaded namespace foo.bar, but no definition of kaocha.plugin/-register found for :foo.bar/baz.

I hope that makes sense... let me know if you have any questions!

@plexus
Copy link
Member Author

plexus commented Nov 2, 2020

Hi @noorfathima11 , I've unassigned you so others can take this up if they want you. Do let me know if you still want to try your hand at this or other issues.

@alysbrooks alysbrooks self-assigned this Dec 2, 2020
@alysbrooks
Copy link
Member

Fixed by #181

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ergonomics Making Kaocha delightful to work with and people more efficient first issue only These tickets are only for first-time contributors. improvement Incremental improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

4 participants