fix(plugins): surface the real reason a plugin bundle fails to load (#1915)#1927
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ca27b73c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try PluginBundleLoader.load(bundle) | ||
| } catch { | ||
| Self.logger.error("Failed to load lazy bundle '\(bundleId)' at \(url.lastPathComponent): \(error.localizedDescription)") | ||
| recordLazyActivationRejection(url: url, bundleId: bundleId, entry: entry, error: error) |
There was a problem hiding this comment.
When a lazy bundle fails after initial startup, this only appends to rejectedPlugins; no AppEvents.shared.pluginsRejected event is sent. PluginNotificationService subscribes exclusively to that event (set up in AppDelegate), while the only sender is the initial reconciliation path that has already run, so the newly added rejected-plugin notification is never delivered for this on-demand failure despite the intended reporting behavior.
Useful? React with 👍 / 👎.
1 participant
Fixes #1915.
Root cause
Bundle.load()returns a bareBooland discards theNSErrorthat dyld/NSBundle produced. Two call sites relied on it, so every distinct load failure (wrong architecture, missing Swift-runtime dylib, incompatible runtime, missing symbol from an ABI break, damaged or unsigned executable) collapsed to one unactionable string: "Invalid plugin bundle: Bundle failed to load executable". The user in #1915 hit this installing Oracle 1.2.17, which weak-linkslibswiftSynchronization.dyliband fails as a missing-dependency load error that today looks identical to an arch mismatch.A second, quieter defect sat next to it: the on-demand activation path only logged the failure and returned, recording no rejected-plugin entry, so a lazily-loaded driver that failed to load just never appeared with no diagnostic anywhere.
What changed
PluginBundleLoaderloads a bundle withBundle.loadAndReturnError()(the documented API that keeps theNSError), logs the full dyld detail via OSLog for support, and translatesNSError.codeinto an actionable, localized reason. It mirrors the existingPluginCodeSignatureVerifier.describeOSStatuspattern. The classification separates:NSExecutableArchitectureMismatchError)NSExecutableRuntimeMismatchError)NSExecutableLoadError, the Oracle case)NSExecutableLinkError, the ABI-break case behind Lỗi sử dụng Mongodb ở bản 0.58 #1917)NSExecutableNotLoadableError)NSFileNoSuchFileError)PluginManagerroutes bothbundle.load()sites throughPluginBundleLoader.load. The install path throws the specific reason up to the existing alerts; the lazy-activation path now records aRejectedPlugininstead of silently returning, so the failure surfaces in the rejected-plugin banner and notification.PluginErroris unchanged:.invalidBundlenow carries the real reason, so every existing alert improves without new UI.The full dyld text (
NSDebugDescriptionErrorKey) goes to OSLog only, per Apple's guidance that it is developer-facing; the user sees the concise classification.Scope
No PluginKit ABI change, so no version bump and no plugin re-release. The issue's second point (Oracle 1.2.17 registry
minAppVersionvs. its build toolchain) is a release-metadata fix in theTableProApp/pluginsrepo, not app code. This change is what makes such mismatches diagnosable.Tests
PluginBundleLoaderTestscovers the code-to-reason translation for all six error codes plus the OS-provided fallback. Deterministic, always runs.PluginLazyActivationVerificationTestsgains a test that builds a bundle with a non-Mach-O executable, activates it, and asserts it is now rejected with a non-generic reason. This guards the silent-swallow fix.Notes for review
swiftlint lint --strictis clean on the changed files. The import reorder inPluginLazyActivationVerificationTestsfixes a pre-existingsorted_importswarning that only shows under--strict.