| Summary: | Github search errors make it impossible to install plugins from other repos | ||
|---|---|---|---|
| Product: | Koha | Reporter: | David Cook <dcook> |
| Component: | Plugin architecture | Assignee: | Jonathan Druart <jonathan.druart> |
| Status: | Pushed to oldstable --- | QA Contact: | Tomás Cohen Arazi (tcohen) <tomascohen> |
| Severity: | normal | ||
| Priority: | P5 - low | CC: | baptiste.wojtkowski, bibliothek, david, jonathan.druart, magnus, tomascohen |
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41032 | ||
| GIT URL: | Initiative type: | --- | |
| Sponsorship status: | --- | Comma delimited list of Sponsors: | |
| Crowdfunding goal: | 0 | Patch complexity: | Small patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: |
This fixes the error 500 "malformed JSON string" message when something goes wrong searching for plugins using the plugin search in the staff interface (for example, when there is an invalid repository in the Koha configuration).
|
Version(s) released in: |
25.11.00,25.05.06,24.11.12
|
| Circulation function: | |||
| Bug Depends on: | 23975 | ||
| Bug Blocks: | |||
| Attachments: |
Bug 25952: Add error-handling to prevent errors from rate limiting
Bug 25952: Skip invalid repositories when searching for plugins Bug 25952: Skip invalid repositories when searching for plugins Bug 25952: Skip invalid repositories when searching for plugins Bug 25952: Skip invalid repositories when searching for plugins |
||
|
Description
David Cook
2020-07-08 10:48:07 UTC
It looks like it's due to rate limiting. I can use the search API: https://api.github.com/search/repositories?q=koha-plugin+user:bywatersolutions+in:name,description But I can't get the releases: https://api.github.com/repos/bywatersolutions/koha-plugin-coverflow/releases/latest I guess that's because I've done < 60 searches, but each search has to do X release API calls, so it would be easy to exceed 60 API calls quickly if a person was looking for "koha-plugin" which would retrieve everything. Created attachment 106650 [details] [review] Bug 25952: Add error-handling to prevent errors from rate limiting This patch makes sure there is a successful response before trying to decode the response. If you search for "koha-plugin" in the Bywater and Theke repos, you'll quickly exceed your API rate limit, and you'll stop getting successful responses. To test: 0) Don't apply patch yet 1) Enable plugins 2) Enable plugin_repos 3) Search for koha-plugin a few times 4) Notice how Plack crashes with a visible error in the browser 5) Apply the patch 6) restart_all (in koha-testing-docker) 7) Search for koha-plugin 8) Observe that no search results are returned and there are no errors in the browser I enabled the feature by adding this to koha-conf.xml:
<plugin_repos>
<repo>
<name>ByWater Solutions</name>
<org_name>bywatersolutions</org_name>
<service>github</service>
</repo>
<repo>
<name>Theke Solutions</name>
<org_name>thekesolutions</org_name>
<service>github</service>
</repo>
</plugin_repos>
Without the patch I always get this error:
malformed JSON string, neither tag, array, object, number, string or atom, at character offset 0 (before "(end of string)") at /usr/share/perl5/JSON.pm line 171
Is this the expected error we are trying to fix here?
After applying the patch I searched for "koha-plugin" and got a bunch of hits, but only from Bywater. When I did the search again straight away it chewed for a while, then I got nothing. That is better than an ugly error message, of course, but wouldn't a friendly message like "oops something went wrong, please adjust your search or try again later" be better?
Perhaps we could also intercept a search for "koha-plugin" and warn that it might exceed the rate limits and give unexepected results?
(In reply to Magnus Enger from comment #3) > I enabled the feature by adding this to koha-conf.xml: > > <plugin_repos> > <repo> > <name>ByWater Solutions</name> > <org_name>bywatersolutions</org_name> > <service>github</service> > </repo> > <repo> > <name>Theke Solutions</name> > <org_name>thekesolutions</org_name> > <service>github</service> > </repo> > </plugin_repos> > > > Without the patch I always get this error: > > malformed JSON string, neither tag, array, object, number, string or atom, > at character offset 0 (before "(end of string)") at /usr/share/perl5/JSON.pm > line 171 > > Is this the expected error we are trying to fix here? > It's been a while but I think so. > After applying the patch I searched for "koha-plugin" and got a bunch of > hits, but only from Bywater. When I did the search again straight away it > chewed for a while, then I got nothing. That is better than an ugly error > message, of course, but wouldn't a friendly message like "oops something > went wrong, please adjust your search or try again later" be better? > > Perhaps we could also intercept a search for "koha-plugin" and warn that it > might exceed the rate limits and give unexepected results? That's a good point, although at this point I think I'm abandoning/shelving my work on this, as I think using the GitHub/GitLab APIs is a flawed approach overall. It does seem a bit brittle if we are already reaching the limits of the API. Do you have another approach in mind or do you think the whole “search and install” feature should be reverted? Setting to “in discussion”. (In reply to Magnus Enger from comment #5) > It does seem a bit brittle if we are already reaching the limits of the API. > Do you have another approach in mind or do you think the whole “search and > install” feature should be reverted? > I don't think that it should be reverted, but I think a manifest based approach would be more robust. It's something I plan to work on eventually one day in the future. Basically, we'd point the Koha config at a set of URLs (which are either for files or directories depending on the implementation) and we'd download manifests of available Koha plugins. (Basically, it's the same model as package managers.) The neat part of that would also be that sysadmins could choose which manifests to point at or even point to a local one of curated plugins. It would be quite flexible. The only downside I see with the manifests is that they would need to be updated, but I think that's really more of a plugin developer issue than a Koha issue. (In reply to David Cook from comment #6) > Basically, we'd point the Koha config at a set of URLs (which are either for > files or directories depending on the implementation) and we'd download > manifests of available Koha plugins. (Basically, it's the same model as > package managers.) Ah, sounds good to me! Created attachment 189584 [details] [review] Bug 25952: Skip invalid repositories when searching for plugins This patch prevents the end-user to get the ugly 500 "malformed JSON string" when something wrong happens with the plugin search. It happens currently in ktd with the plugin repos listed in $KOHA_CONF An error is reported to the UI Created attachment 189585 [details] [review] Bug 25952: Skip invalid repositories when searching for plugins This patch prevents the end-user to get the ugly 500 "malformed JSON string" when something wrong happens with the plugin search. It happens currently in ktd with the plugin repos listed in $KOHA_CONF An error is reported to the UI This patch fixes a plugin that appears in ktd recently (we added a list of plugin repos, and ptfs-e's one is wrong). I think it's ready for SO even if not perfect. It replaces David's patch (I hadn't seen it before). Feel free to obsolete if you disagree. Created attachment 189597 [details] [review] Bug 25952: Skip invalid repositories when searching for plugins This patch prevents the end-user to get the ugly 500 "malformed JSON string" when something wrong happens with the plugin search. It happens currently in ktd with the plugin repos listed in $KOHA_CONF An error is reported to the UI Signed-off-by: Owen Leonard <oleonard@myacpl.org> With the patch applied: I just ran ktd with the --plugins option which enables the following repos:
<plugin_repos>
<repo>
<name>ByWater Solutions</name>
<org_name>bywatersolutions</org_name>
<service>github</service>
</repo>
<repo>
<name>Theke Solutions</name>
<org_name>thekesolutions</org_name>
<service>github</service>
</repo>
<repo>
<name>PTFS Europe</name>
<org_name>PTFS-Europe</org_name>
<service>github</service>
</repo>
<repo>
<name>Solutions inLibro</name>
<org_name>inLibro</org_name>
<service>github</service>
</repo>
</plugin_repos>
Then I searched for koha-plugin plugins-home.pl and it only returned a result for PTFS
Cannot search repository PTFS Europe: malformed JSON string, neither tag, array, object, number, string or atom, at character offset 0 (before "(end of string)") at /usr/share/perl5/JSON.pm line 190.
I did not get any other error for the other repos.
This just calls for the proposed plugin store Open Fifth presented :)
(In reply to Jan Kissig from comment #12) > Then I searched for koha-plugin plugins-home.pl and it only returned a > result for PTFS > > Cannot search repository PTFS Europe: malformed JSON string, neither tag, > array, object, number, string or atom, at character offset 0 (before "(end > of string)") at /usr/share/perl5/JSON.pm line 190. > I did not get any other error for the other repos. That error should go away if you delete or update the config for the PTFS Europe repo, ref bug 41032. Created attachment 189848 [details] [review] Bug 25952: Skip invalid repositories when searching for plugins This patch prevents the end-user to get the ugly 500 "malformed JSON string" when something wrong happens with the plugin search. It happens currently in ktd with the plugin repos listed in $KOHA_CONF An error is reported to the UI Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Tested using the default (wrong) config provided by KTD, which still ships with `PTFS-Europe`. The result is the expected: 1. Search for 'rapido' on the plugins page => SUCCESS: A warning about an error searching PTFS Europe => SUCCESS: The `koha-plugin-rapido-ill` plugin is displayed and can be installed Nice work everyone! Pushed to main for 25.11 Nice work everyone! Pushed to 25.05.x Pushed into 24.11.x for 24.11.11 nice work everyone Pushed into 24.11.x for 24.11.12 nice work everyone |