Skip to content

ENH: Better error catching when loading plugins#6877

Merged
TEParsons merged 2 commits intopsychopy:devfrom
TEParsons:dev-enh-plugin-logging
Jan 3, 2025
Merged

ENH: Better error catching when loading plugins#6877
TEParsons merged 2 commits intopsychopy:devfrom
TEParsons:dev-enh-plugin-logging

Conversation

@TEParsons
Copy link
Copy Markdown
Contributor

Currently, because we use return False rather than continue when an entry point fails to load, if one entry point fails then none of the rest of the entry points for that plugin are registered. There also isn't any detail given, so the logs just look like:

ERROR Failed to load <entry point> from <plugin-name> for unknown reason. Skipping

This PR means that when an entry point fails, only that specific entry point is skipped, and we'll log the reason.

@TEParsons TEParsons requested a review from mdcutone September 26, 2024 10:01
@codecov
Copy link
Copy Markdown

codecov bot commented Sep 26, 2024

Codecov Report

Attention: Patch coverage is 20.00000% with 8 lines in your changes missing coverage. Please review.

Project coverage is 50.60%. Comparing base (1565e6b) to head (f3bbeb0).
Report is 31 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #6877      +/-   ##
==========================================
- Coverage   50.87%   50.60%   -0.28%     
==========================================
  Files         342      347       +5     
  Lines       62401    62856     +455     
==========================================
+ Hits        31748    31806      +58     
- Misses      30653    31050     +397     
Components Coverage Δ
app ∅ <ø> (∅)
boilerplate ∅ <ø> (∅)
library ∅ <ø> (∅)
vm-safe library ∅ <ø> (∅)

@mdcutone
Copy link
Copy Markdown
Member

mdcutone commented Sep 26, 2024

Wouldn't this result in partially loaded plugins? Should we want to stop loading entry points for a specific plugin if there is a fail and warn the user? Partial loading of plugins could lead to undefined behaviour. I agree the logging needs to be improved around this.

@TEParsons
Copy link
Copy Markdown
Contributor Author

It would, but I think a partially loaded plugin is better than an aborted plugin. For example, the thing that motivated this was the Cedrus plugin failing to load due to an error in Riponda VoiceKey - meaning the old Cedrus Button Box Component wasn't loading, despite it not being at all dependent on the Riponda backend. If an entry point can still load without error I think we should allow it to, even if there's a problem with another entry point in the same plugin.

@TEParsons TEParsons force-pushed the dev-enh-plugin-logging branch from 6da22f5 to f3bbeb0 Compare January 2, 2025 14:59
@TEParsons TEParsons merged commit 43ecc81 into psychopy:dev Jan 3, 2025
@TEParsons TEParsons deleted the dev-enh-plugin-logging branch February 12, 2025 16:58
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