plugins: ensure to log the offending plugin on instantiation failure #5739
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry. |
1 task
snejus
reviewed
Apr 18, 2025
wisp3rwind
force-pushed
the
plugin-traceback
branch
from
3a4a4b7 to
db32fa1
Compare
wisp3rwind
force-pushed
the
plugin-traceback
branch
5 times, most recently
from
bde4397 to
2129eb4
Compare
|
Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry. |
|
This should be ready to go now; it's easiest to review commit-by-commit. |
wisp3rwind
force-pushed
the
plugin-traceback
branch
from
2129eb4 to
cc877e5
Compare
minimize how much code is wrapped in a given try...except statement
…roken error handling from a quick test, it appears that the old code shouldn't even be working anymore due to additional quotes in exc.args[0]
also remove a unnecessary check before adding to a set
without an argument (i.e. the default `names=()`), this was a no-op actual plugin loading is triggered by tests, or by the ui code -> Turns out it's not an actual no-op in our testing code due to patching with attaching some side-effects to load_plugins... This is fixed up in the next commit.
Instead of the first call to find_plugins(). This could change behavior slightly, since plugin load (and errors that happen in the process) could occur earlier now. But: - I'd argue this is better anyway, since plugin load errors now happen in the initial call to load_plugins() instead of somewhere in the middle of the first operation that uses plugins. - In our ui code, both happen immediately after each other anyway: ui._setup calls 1) ui._load_plugins -> plugins.load_plugins 2) plugins.types, plugins.named_queries, ...
wisp3rwind
force-pushed
the
plugin-traceback
branch
from
cc877e5 to
8a862ba
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Came up in #5701 (comment) where an error can't easily be traced back to a plugin.
(Draft because I want to avoid creating merge conflicts for #5701 before that is merged
, and because I might a few additional improvements here.)