From c665ad8e0c7af3b960648c98fdcf99b2d92057e1 Mon Sep 17 00:00:00 2001 From: Grace Kind Date: Wed, 13 May 2026 15:05:21 -0500 Subject: [PATCH] Disable plugins on load failure --- package.json | 2 +- src/js/components/toggle-switch.js | 4 +- src/js/plugins/pluginBridge.js | 17 ++- src/js/plugins/pluginRendering.js | 10 +- src/js/plugins/pluginService.js | 126 ++++++++++++------ .../views/settings/communityPlugins.view.js | 13 +- src/js/views/settings/plugins.view.js | 53 ++++++-- 7 files changed, 160 insertions(+), 65 deletions(-) diff --git a/package.json b/package.json index 464e13fc..e4436820 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "impro", - "version": "0.14.10", + "version": "0.14.11", "type": "module", "scripts": { "start": "rm -rf build && NODE_ENV=development eleventy --serve", diff --git a/src/js/components/toggle-switch.js b/src/js/components/toggle-switch.js index 5bd58f24..628b841a 100644 --- a/src/js/components/toggle-switch.js +++ b/src/js/components/toggle-switch.js @@ -47,7 +47,6 @@ class ToggleSwitch extends Component { render() { const label = this.getAttribute("label") ?? ""; - render( html`
{ if (this.disabled) return; - this.checked = !this.checked; this.dispatchEvent( new CustomEvent("change", { - detail: { checked: this.checked }, + detail: { checked: !this.checked }, bubbles: true, }), ); diff --git a/src/js/plugins/pluginBridge.js b/src/js/plugins/pluginBridge.js index b92ec2a5..0fa10f09 100644 --- a/src/js/plugins/pluginBridge.js +++ b/src/js/plugins/pluginBridge.js @@ -237,9 +237,22 @@ export class PluginBridge { } async loadPlugins(pluginRequests) { + const loadedPlugins = []; + const erroredPlugins = []; await Promise.all( - pluginRequests.map(({ id, version }) => this.loadPlugin(id, version)), + pluginRequests.map(async ({ id, version }) => { + try { + const plugin = await this.loadPlugin(id, version); + loadedPlugins.push(plugin); + } catch (error) { + erroredPlugins.push({ pluginId: id, version, error }); + } + }), ); + return { + loadedPlugins, + erroredPlugins, + }; } async loadPlugin(pluginId, version) { @@ -273,8 +286,10 @@ export class PluginBridge { ); this._loadedPlugins.set(pluginId, pluginInstance); logger.info(`loaded "${pluginId}" v${manifest.version}`); + return pluginInstance; } catch (error) { logger.error(`"${pluginId}" failed during initialization:`, error); + throw error; } } diff --git a/src/js/plugins/pluginRendering.js b/src/js/plugins/pluginRendering.js index 27270aba..7cdc154d 100644 --- a/src/js/plugins/pluginRendering.js +++ b/src/js/plugins/pluginRendering.js @@ -91,10 +91,18 @@ export class PluginRenderer { ); tag = "span"; } - if (tag === "input" && node.attrs?.type === "checkbox") { + const aliasedAsToggle = tag === "input" && node.attrs?.type === "checkbox"; + if (aliasedAsToggle) { tag = "toggle-switch"; } const element = document.createElement(tag); + if (aliasedAsToggle) { + // toggle-switch is controlled — flip its state here since the plugin + // worker can't observe events synchronously to re-render. + element.addEventListener("change", (e) => { + element.checked = e.detail?.checked ?? !element.checked; + }); + } if (node.attrs) { for (const [name, value] of Object.entries(node.attrs)) { if (!isAllowedAttr(name)) { diff --git a/src/js/plugins/pluginService.js b/src/js/plugins/pluginService.js index 2667410f..650fbfbb 100644 --- a/src/js/plugins/pluginService.js +++ b/src/js/plugins/pluginService.js @@ -1,6 +1,6 @@ import { PluginBridge } from "/js/plugins/pluginBridge.js"; import { showPluginModal, hidePluginModal } from "/js/modals.js"; -import { showPluginToast, hidePluginToast } from "/js/toasts.js"; +import { showPluginToast, hidePluginToast, showToast } from "/js/toasts.js"; import { PluginRenderer } from "/js/plugins/pluginRendering.js"; import { PluginRegistry } from "/js/plugins/pluginRegistry.js"; import { PluginCache } from "/js/plugins/pluginCache.js"; @@ -181,9 +181,9 @@ export class PluginService extends EventEmitter { } async loadPluginsInfo() { - const installed = this._getInstalled(); + const installedPluginsPreference = this._getInstalledPluginsPreference(); const results = await Promise.all( - installed.map(async (entry) => { + installedPluginsPreference.map(async (entry) => { const manifest = await this.sourceProvider.ensureManifest( entry.id, entry.version, @@ -202,22 +202,36 @@ export class PluginService extends EventEmitter { } async loadEnabledPlugins() { - const installed = this._getInstalled(); - const toLoad = installed.filter((entry) => entry.enabled); - await this.pluginBridge.loadPlugins(toLoad); + const installedPluginsPreference = this._getInstalledPluginsPreference(); + const toLoad = installedPluginsPreference.filter((entry) => entry.enabled); + const { erroredPlugins } = await this.pluginBridge.loadPlugins(toLoad); + if (erroredPlugins.length) { + const failedPluginIds = erroredPlugins.map(({ pluginId }) => pluginId); + showToast(`Failed to load plugin(s): ${failedPluginIds.join(", ")}`, { + style: "error", + }); + // Disable plugins that failed to load + await Promise.all( + failedPluginIds.map((pluginId) => this._setPluginDisabled(pluginId)), + ); + } // Reconcile against all installed plugins (not just enabled) so disabled // plugins keep their cached assets on re-enable - await this._reconcileCache(installed); + await this._reconcileCache(installedPluginsPreference); } async getManifest(pluginId) { - const installed = this._getInstalled().find( - (plugin) => plugin.id === pluginId, + const installedPluginsPreference = + this._getInstalledPluginsPreference().find( + (plugin) => plugin.id === pluginId, + ); + return this.sourceProvider.ensureManifest( + pluginId, + installedPluginsPreference?.version, ); - return this.sourceProvider.ensureManifest(pluginId, installed?.version); } - _getInstalled() { + _getInstalledPluginsPreference() { if (!this.preferencesProvider) return []; try { return this.preferencesProvider @@ -228,7 +242,7 @@ export class PluginService extends EventEmitter { } } - async _setInstalled(plugins) { + async _setInstalledPluginsPreference(plugins) { const preferences = this.preferencesProvider .requirePreferences() .setInstalledPlugins(plugins); @@ -237,19 +251,19 @@ export class PluginService extends EventEmitter { // TODO // async _applyAutoUpdates() { - // const installed = this._getInstalled(); - // if (installed.length === 0) return installed; + // const installedPluginsPreference = this._getInstalledPluginsPreference(); + // if (installedPluginsPreference.length === 0) return installedPluginsPreference; // let listings; // try { // listings = await this.registry.getPluginListings(); // } catch { - // return installed; + // return installedPluginsPreference; // } // const listingById = new Map( // listings.map((listing) => [listing.id, listing]), // ); // const liveVersions = await Promise.all( - // installed.map(async (entry) => { + // installedPluginsPreference.map(async (entry) => { // const listing = listingById.get(entry.id); // if (!listing || listing.local) return null; // try { @@ -261,7 +275,7 @@ export class PluginService extends EventEmitter { // }), // ); // let changed = false; - // const next = installed.map((entry, index) => { + // const next = installedPluginsPreference.map((entry, index) => { // const liveVersion = liveVersions[index]; // if (!liveVersion) return entry; // if (compareVersions(liveVersion, entry.version) > 0) { @@ -270,7 +284,7 @@ export class PluginService extends EventEmitter { // } // return entry; // }); - // if (changed) await this._setInstalled(next); + // if (changed) await this._setInstalledPluginsPreference(next); // return next; // } @@ -288,13 +302,14 @@ export class PluginService extends EventEmitter { if (!listing) { throw new Error(`unknown plugin: ${pluginId}`); } - const installed = this._getInstalled(); - if (installed.some((plugin) => plugin.id === pluginId)) return; + const installedPluginsPreference = this._getInstalledPluginsPreference(); + if (installedPluginsPreference.some((plugin) => plugin.id === pluginId)) + return; const version = listing.local ? (await this.sourceProvider.getManifest(pluginId)).version : (await this.registry.fetchLiveManifest(listing)).version; - await this._setInstalled([ - ...installed, + await this._setInstalledPluginsPreference([ + ...installedPluginsPreference, { id: pluginId, version, enabled: true }, ]); await this.pluginBridge.loadPlugin(pluginId, version); @@ -302,32 +317,51 @@ export class PluginService extends EventEmitter { async uninstallPlugin(pluginId) { this.pluginBridge.unloadPlugin(pluginId); - const next = this._getInstalled().filter( + const next = this._getInstalledPluginsPreference().filter( (plugin) => plugin.id !== pluginId, ); - await this._setInstalled(next); + await this._setInstalledPluginsPreference(next); await this._reconcileCache(next); } async enablePlugin(pluginId) { - const installed = this._getInstalled(); - const entry = installed.find((plugin) => plugin.id === pluginId); + await this._setPluginEnabled(pluginId); + const entry = this._getInstalledPluginsPreference().find( + (plugin) => plugin.id === pluginId, + ); + try { + await this.pluginBridge.loadPlugin(pluginId, entry.version); + } catch (e) { + await this._setPluginDisabled(pluginId); + throw e; + } + } + + async _setPluginEnabled(pluginId) { + const installedPluginsPreference = this._getInstalledPluginsPreference(); + const entry = installedPluginsPreference.find( + (plugin) => plugin.id === pluginId, + ); if (!entry) throw new Error(`not installed: ${pluginId}`); if (entry.enabled) return; - await this._setInstalled( - installed.map((plugin) => + await this._setInstalledPluginsPreference( + installedPluginsPreference.map((plugin) => plugin.id === pluginId ? { ...plugin, enabled: true } : plugin, ), ); - await this.pluginBridge.loadPlugin(pluginId, entry.version); } async disablePlugin(pluginId) { this.pluginBridge.unloadPlugin(pluginId); - const installed = this._getInstalled(); - if (!installed.some((plugin) => plugin.id === pluginId)) return; - await this._setInstalled( - installed.map((plugin) => + await this._setPluginDisabled(pluginId); + } + + async _setPluginDisabled(pluginId) { + const installedPluginsPreference = this._getInstalledPluginsPreference(); + if (!installedPluginsPreference.some((plugin) => plugin.id === pluginId)) + return; + await this._setInstalledPluginsPreference( + installedPluginsPreference.map((plugin) => plugin.id === pluginId ? { ...plugin, enabled: false } : plugin, ), ); @@ -336,20 +370,26 @@ export class PluginService extends EventEmitter { async checkForUpdates(pluginId) { const listing = await this.registry.getPluginListing(pluginId); if (!listing || listing.local) return null; - const installed = this._getInstalled().find( - (plugin) => plugin.id === pluginId, - ); - if (!installed) return null; + const installedPluginsPreference = + this._getInstalledPluginsPreference().find( + (plugin) => plugin.id === pluginId, + ); + if (!installedPluginsPreference) return null; const liveManifest = await this.registry.fetchLiveManifest(listing); - if (compareVersions(liveManifest.version, installed.version) > 0) { + if ( + compareVersions( + liveManifest.version, + installedPluginsPreference.version, + ) > 0 + ) { // Re-read after the await and merge by id so a concurrent // enable/disable/uninstall isn't clobbered. - const next = this._getInstalled().map((plugin) => + const next = this._getInstalledPluginsPreference().map((plugin) => plugin.id === pluginId ? { ...plugin, version: liveManifest.version } : plugin, ); - await this._setInstalled(next); + await this._setInstalledPluginsPreference(next); return { updated: true, version: liveManifest.version }; } return { updated: false }; @@ -357,7 +397,9 @@ export class PluginService extends EventEmitter { async listRegistryPlugins() { const listings = await this.registry.getPluginListings(); - const installedIds = new Set(this._getInstalled().map((entry) => entry.id)); + const installedIds = new Set( + this._getInstalledPluginsPreference().map((entry) => entry.id), + ); return listings.map((listing) => ({ ...listing, installed: installedIds.has(listing.id), @@ -365,7 +407,7 @@ export class PluginService extends EventEmitter { } getEnabledPlugins() { - return this._getInstalled() + return this._getInstalledPluginsPreference() .filter((entry) => entry.enabled) .map((entry) => entry.id); } diff --git a/src/js/views/settings/communityPlugins.view.js b/src/js/views/settings/communityPlugins.view.js index f08f3b59..d65b8e0c 100644 --- a/src/js/views/settings/communityPlugins.view.js +++ b/src/js/views/settings/communityPlugins.view.js @@ -61,15 +61,18 @@ class SettingsCommunityPluginsView extends View { await pluginService.installPlugin(entry.id); } state.entries = await pluginService.listRegistryPlugins(); - showToast(wasInstalled ? "Uninstalled plugin" : "Installed plugin", { - style: wasInstalled ? "default" : "success", - }); + showToast( + wasInstalled + ? `Uninstalled ${entry.name}` + : `Installed ${entry.name}`, + { style: wasInstalled ? "default" : "success" }, + ); } catch (error) { state.error = error.message ?? String(error); showToast( wasInstalled - ? "Failed to uninstall plugin" - : "Failed to install plugin", + ? `Failed to uninstall ${entry.name}` + : `Failed to install ${entry.name}`, { style: "error" }, ); } diff --git a/src/js/views/settings/plugins.view.js b/src/js/views/settings/plugins.view.js index db43ec7f..09dbec8c 100644 --- a/src/js/views/settings/plugins.view.js +++ b/src/js/views/settings/plugins.view.js @@ -26,6 +26,8 @@ class SettingsPluginsView extends View { const state = { uninstallingIds: new Set(), + enablingIds: new Set(), + disablingIds: new Set(), }; async function loadPlugins() { @@ -48,7 +50,7 @@ class SettingsPluginsView extends View { try { await pluginService.uninstallPlugin(plugin.id); await loadPlugins(); - showToast(`Uninstalled ${plugin.manifest.name}`, { style: "success" }); + showToast(`Uninstalled ${plugin.manifest.name}`); } finally { state.uninstallingIds.delete(plugin.id); renderPage(); @@ -56,12 +58,28 @@ class SettingsPluginsView extends View { } async function togglePlugin(plugin) { - if (plugin.enabled) { - await pluginService.disablePlugin(plugin.id); - } else { - await pluginService.enablePlugin(plugin.id); + const pendingSet = plugin.enabled + ? state.disablingIds + : state.enablingIds; + pendingSet.add(plugin.id); + renderPage(); + try { + if (plugin.enabled) { + await pluginService.disablePlugin(plugin.id); + showToast(`Disabled ${plugin.manifest.name}`); + } else { + try { + await pluginService.enablePlugin(plugin.id); + showToast(`Enabled ${plugin.manifest.name}`, { style: "success" }); + } catch (e) { + showToast("Plugin failed to load", { style: "error" }); + } + } + await loadPlugins(); + } finally { + pendingSet.delete(plugin.id); + renderPage(); } - await loadPlugins(); } function renderPage() { @@ -119,15 +137,19 @@ class SettingsPluginsView extends View {

` : html``} `, })} -- 2.51.2