diff --git a/frontend/src/app/main/data/plugins.cljs b/frontend/src/app/main/data/plugins.cljs index e04aecbf7c..b091518b67 100644 --- a/frontend/src/app/main/data/plugins.cljs +++ b/frontend/src/app/main/data/plugins.cljs @@ -7,12 +7,14 @@ (ns app.main.data.plugins (:require [app.common.data.macros :as dm] + [app.common.exceptions :as ex] [app.common.files.changes-builder :as pcb] [app.common.time :as ct] [app.main.data.changes :as dch] [app.main.data.event :as ev] [app.main.data.modal :as modal] [app.main.data.notifications :as ntf] + [app.main.errors :as errors] [app.main.store :as st] [app.plugins.flags :as pflag] [app.plugins.register :as preg] @@ -20,7 +22,8 @@ [app.util.http :as http] [app.util.i18n :as i18n :refer [tr]] [beicon.v2.core :as rx] - [potok.v2.core :as ptk])) + [potok.v2.core :as ptk] + [promesa.core :as p])) (defn save-plugin-permissions-peek [id permissions] @@ -54,40 +57,45 @@ (defn start-plugin! [{:keys [plugin-id name version description host code permissions allow-background]} ^js extensions] - (.ɵloadPlugin - ^js ug/global - #js {:pluginId plugin-id - :name name - :version version - :description description - :host host - :code code - :allowBackground (boolean allow-background) - :permissions (apply array permissions)} - nil - extensions)) + (-> (.ɵloadPlugin + ^js ug/global + #js {:pluginId plugin-id + :name name + :version version + :description description + :host host + :code code + :allowBackground (boolean allow-background) + :permissions (apply array permissions)} + nil + extensions) + + (p/catch (fn [cause] + (ex/print-throwable cause :prefix "Plugin Error") + (errors/flash :cause cause :type :handled))))) (defn- load-plugin! [{:keys [plugin-id name version description host code icon permissions]}] - (try - (st/emit! (pflag/clear plugin-id) - (save-current-plugin plugin-id)) + (st/emit! (pflag/clear plugin-id) + (save-current-plugin plugin-id)) - (.ɵloadPlugin ^js ug/global - #js {:pluginId plugin-id - :name name - :description description - :version version - :host host - :code code - :icon icon - :permissions (apply array permissions)} - (fn [] - (st/emit! (remove-current-plugin plugin-id)))) + (-> (.ɵloadPlugin + ^js ug/global + #js {:pluginId plugin-id + :name name + :description description + :version version + :host host + :code code + :icon icon + :permissions (apply array permissions)} + (fn [] + (st/emit! (remove-current-plugin plugin-id)))) - (catch :default e - (st/emit! (remove-current-plugin plugin-id)) - (.error js/console "Error" e)))) + (p/catch (fn [cause] + (st/emit! (remove-current-plugin plugin-id)) + (ex/print-throwable cause :prefix "Plugin Error") + (errors/flash :cause cause :type :handled))))) (defn open-plugin! [{:keys [url] :as manifest} user-can-edit?] diff --git a/frontend/src/app/main/errors.cljs b/frontend/src/app/main/errors.cljs index 4eb52b5b35..49f4080abb 100644 --- a/frontend/src/app/main/errors.cljs +++ b/frontend/src/app/main/errors.cljs @@ -33,6 +33,16 @@ ;; Will contain last uncaught exception (def last-exception nil) +(defn is-plugin-error? + "This is a placeholder that always return false. It will be + overwritten when plugin system is initialized. This works this way + because we can't import plugins here because plugins requries full + DOM. + + This placeholder is set on app.plugins/initialize event" + [_] + false) + ;; --- Stale-asset error detection and auto-reload ;; ;; When the browser loads JS modules from different builds (e.g. shared.js from @@ -387,6 +397,15 @@ (and (string? stack) (str/includes? stack "posthog")))) + ;; Check if the error is marked as originating from plugin code. + ;; The plugin runtime tracks plugin errors in a WeakMap, which works + ;; even in SES hardened environments where error objects may be frozen. + (from-plugin? [cause] + (try + (is-plugin-error? cause) + (catch :default _ + false))) + (is-ignorable-exception? [cause] (let [message (ex-message cause)] (or (from-extension? cause) @@ -405,32 +424,56 @@ (on-unhandled-error [event] (.preventDefault ^js event) (when-let [cause (unchecked-get event "error")] - (when-not (is-ignorable-exception? cause) - (if (stale-asset-error? cause) - (cf/throttled-reload :reason (ex-message cause)) - (let [data (ex-data cause) - type (get data :type)] - (set! last-exception cause) - (if (= :wasm-error type) - (on-error cause) - (do - (ex/print-throwable cause :prefix "Uncaught Exception") - (ts/asap #(flash :cause cause :type :unhandled))))))))) + (cond + (stale-asset-error? cause) + (cf/throttled-reload :reason (ex-message cause)) + + ;; Plugin errors: log to console and ignore + (from-plugin? cause) + (ex/print-throwable cause :prefix "Plugin Error") + + ;; Other ignorable exceptions: ignore silently + (is-ignorable-exception? cause) + nil + + ;; All other errors: show exception page + :else + + (let [data (ex-data cause) + type (get data :type)] + (set! last-exception cause) + (if (= :wasm-error type) + (on-error cause) + (do + (ex/print-throwable cause :prefix "Uncaught Exception") + (ts/asap #(flash :cause cause :type :unhandled)))))))) (on-unhandled-rejection [event] (.preventDefault ^js event) (when-let [cause (unchecked-get event "reason")] - (when-not (is-ignorable-exception? cause) - (if (stale-asset-error? cause) - (cf/throttled-reload :reason (ex-message cause)) - (let [data (ex-data cause) - type (get data :type)] - (set! last-exception cause) - (if (= :wasm-error type) - (on-error cause) - (do - (ex/print-throwable cause :prefix "Uncaught Rejection") - (ts/asap #(flash :cause cause :type :unhandled)))))))))] + (cond + (stale-asset-error? cause) + (cf/throttled-reload :reason (ex-message cause)) + + ;; Plugin errors: log to console and ignore + (from-plugin? cause) + (ex/print-throwable cause :prefix "Plugin Error") + + ;; Other ignorable exceptions: ignore silently + (is-ignorable-exception? cause) + nil + + ;; All other errors: show exception page + :else + (let [data (ex-data cause) + type (get data :type)] + (set! last-exception cause) + (if (= :wasm-error type) + (on-error cause) + (do + (ex/print-throwable cause :prefix "Uncaught Rejection") + (ts/asap #(flash :cause cause :type :unhandled))))))))] + (.addEventListener g/window "error" on-unhandled-error) (.addEventListener g/window "unhandledrejection" on-unhandled-rejection) diff --git a/frontend/src/app/plugins.cljs b/frontend/src/app/plugins.cljs index 36f31ff110..3ce022753e 100644 --- a/frontend/src/app/plugins.cljs +++ b/frontend/src/app/plugins.cljs @@ -8,6 +8,7 @@ "RPC for plugins runtime." (:require ["@penpot/plugins-runtime" :as runtime] + [app.main.errors :as errors] [app.main.features :as features] [app.main.store :as st] [app.plugins.api :as api] @@ -30,6 +31,8 @@ (ptk/reify ::initialize ptk/WatchEvent (watch [_ _ stream] + (set! errors/is-plugin-error? runtime/isPluginError) + (->> stream (rx/filter (ptk/type? ::features/initialize)) (rx/observe-on :async) diff --git a/plugins/apps/poc-state-plugin/src/app/app.component.ts b/plugins/apps/poc-state-plugin/src/app/app.component.ts index 8996e1eb83..0eae67b448 100644 --- a/plugins/apps/poc-state-plugin/src/app/app.component.ts +++ b/plugins/apps/poc-state-plugin/src/app/app.component.ts @@ -393,7 +393,7 @@ export class AppComponent { } #startDownload(name: string, data: Uint8Array) { - const blob = new Blob([data], { type: 'application/octet-stream' }); + const blob = new Blob([data as any], { type: 'application/octet-stream' }); // We need to start a download with this URL const downloadURL = URL.createObjectURL(blob); diff --git a/plugins/libs/plugins-runtime/src/index.ts b/plugins/libs/plugins-runtime/src/index.ts index 2516cfb2f2..a38203471d 100644 --- a/plugins/libs/plugins-runtime/src/index.ts +++ b/plugins/libs/plugins-runtime/src/index.ts @@ -8,6 +8,9 @@ import { ɵunloadPlugin, } from './lib/load-plugin.js'; +// Export the plugin error checker so the frontend can identify plugin errors +export { isPluginError } from './lib/create-sandbox.js'; + import type { Context } from '@penpot/plugin-types'; console.log('%c[PLUGINS] Loading plugin system', 'color: #008d7c'); diff --git a/plugins/libs/plugins-runtime/src/lib/create-plugin.spec.ts b/plugins/libs/plugins-runtime/src/lib/create-plugin.spec.ts index 70d9e74ad4..29d0b6d691 100644 --- a/plugins/libs/plugins-runtime/src/lib/create-plugin.spec.ts +++ b/plugins/libs/plugins-runtime/src/lib/create-plugin.spec.ts @@ -11,6 +11,7 @@ vi.mock('./plugin-manager.js', () => ({ vi.mock('./create-sandbox.js', () => ({ createSandbox: vi.fn(), + markPluginError: vi.fn(), })); describe('createPlugin', () => { @@ -116,7 +117,11 @@ describe('createPlugin', () => { throw new Error('Evaluation error'); }); - await createPlugin(mockContext, manifest, onCloseCallback); + try { + await createPlugin(mockContext, manifest, onCloseCallback); + } catch (err) { + expect.assert(err); + } expect(mockPluginManager.close).toHaveBeenCalled(); }); diff --git a/plugins/libs/plugins-runtime/src/lib/create-plugin.ts b/plugins/libs/plugins-runtime/src/lib/create-plugin.ts index f30977d5aa..fd7bc6a0a8 100644 --- a/plugins/libs/plugins-runtime/src/lib/create-plugin.ts +++ b/plugins/libs/plugins-runtime/src/lib/create-plugin.ts @@ -1,7 +1,7 @@ import type { Context } from '@penpot/plugin-types'; import type { Manifest } from './models/manifest.model.js'; import { createPluginManager } from './plugin-manager.js'; -import { createSandbox } from './create-sandbox.js'; +import { createSandbox, markPluginError } from './create-sandbox.js'; export async function createPlugin( context: Context, @@ -13,9 +13,9 @@ export async function createPlugin( try { sandbox.evaluate(); } catch (error) { - console.error(error); - + markPluginError(error); plugin.close(); + throw error; } }; @@ -33,7 +33,7 @@ export async function createPlugin( const sandbox = createSandbox(plugin, apiExtensions); - evaluateSandbox(); + await evaluateSandbox(); return { plugin, diff --git a/plugins/libs/plugins-runtime/src/lib/create-sandbox.ts b/plugins/libs/plugins-runtime/src/lib/create-sandbox.ts index 01f132548b..292ae49183 100644 --- a/plugins/libs/plugins-runtime/src/lib/create-sandbox.ts +++ b/plugins/libs/plugins-runtime/src/lib/create-sandbox.ts @@ -3,6 +3,61 @@ import type { createPluginManager } from './plugin-manager'; import { createApi } from './api'; import { ses } from './ses.js'; +/** + * WeakMap used to track errors originating from plugin code. + * Using a WeakMap is safer than extending error objects because: + * 1. It works even if the error object is frozen (SES hardened environment) + * 2. It doesn't require modifying the error object + * 3. It allows garbage collection of error objects when no longer referenced + */ +const pluginErrors = new WeakMap(); + +/** + * Checks if an error originated from plugin code. + */ +export function isPluginError(error: unknown): boolean { + if (error !== null && typeof error === 'object') { + return pluginErrors.has(error as object); + } + return false; +} + +/** + * Marks an error as originating from plugin code. + * Uses a WeakMap so it works even if the error object is frozen. + */ +export function markPluginError(error: unknown): void { + if (error !== null && typeof error === 'object') { + pluginErrors.set(error as object, true); + } +} + +/** + * Wraps a handler function to mark any thrown errors as plugin errors. + * Errors are marked and re-thrown so they propagate to the global error handler, + * where they can be identified and handled appropriately. + */ +function wrapHandler unknown>( + handler: T, +): (...args: Parameters) => ReturnType { + return function (...args: Parameters) { + try { + const result = handler(...args); + // Handle async functions - mark errors in the returned promise + if (result instanceof Promise) { + return result.catch((error: unknown) => { + markPluginError(error); + throw error; + }) as ReturnType; + } + return result as ReturnType; + } catch (error) { + markPluginError(error); + throw error; + } + }; +} + export function createSandbox( plugin: Awaited>, apiExtensions?: object, @@ -58,9 +113,10 @@ export function createSandbox( fetch: ses.harden(safeFetch), setTimeout: ses.harden( (...[handler, timeout]: Parameters) => { - const timeoutId = setTimeout(() => { - handler(); - }, timeout); + const wrappedHandler = wrapHandler( + typeof handler === 'function' ? handler : () => {}, + ); + const timeoutId = setTimeout(wrappedHandler, timeout); plugin.timeouts.add(timeoutId); @@ -72,6 +128,23 @@ export function createSandbox( plugin.timeouts.delete(id); }), + setInterval: ses.harden( + (...[handler, interval]: Parameters) => { + const wrappedHandler = wrapHandler( + typeof handler === 'function' ? handler : () => {}, + ); + const intervalId = setInterval(wrappedHandler, interval); + + plugin.intervals.add(intervalId); + + return ses.safeReturn(intervalId); + }, + ) as typeof setInterval, + clearInterval: ses.harden((id: ReturnType) => { + clearInterval(id); + + plugin.intervals.delete(id); + }), /** * GLOBAL FUNCTIONS ACCESIBLE TO PLUGINS diff --git a/plugins/libs/plugins-runtime/src/lib/load-plugin.spec.ts b/plugins/libs/plugins-runtime/src/lib/load-plugin.spec.ts index a46bbc8573..cff0f5e57a 100644 --- a/plugins/libs/plugins-runtime/src/lib/load-plugin.spec.ts +++ b/plugins/libs/plugins-runtime/src/lib/load-plugin.spec.ts @@ -19,6 +19,10 @@ vi.mock('./create-plugin', () => ({ createPlugin: vi.fn(), })); +vi.mock('./create-sandbox.js', () => ({ + markPluginError: vi.fn(), +})); + vi.mock('./ses.js', () => ({ ses: { harden: vi.fn().mockImplementation((obj) => obj), @@ -102,16 +106,17 @@ describe('plugin-loader', () => { }); it('should handle errors and close all plugins', async () => { - const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - vi.mocked(createPlugin).mockRejectedValue( new Error('Plugin creation failed'), ); - await loadPlugin(manifest); + try { + await loadPlugin(manifest); + } catch (err) { + expect.assert(err); + } expect(getPlugins()).toHaveLength(0); - expect(consoleSpy).toHaveBeenCalled(); }); it('should handle messages sent to plugins', async () => { diff --git a/plugins/libs/plugins-runtime/src/lib/load-plugin.ts b/plugins/libs/plugins-runtime/src/lib/load-plugin.ts index 990a9148a1..8a88f23050 100644 --- a/plugins/libs/plugins-runtime/src/lib/load-plugin.ts +++ b/plugins/libs/plugins-runtime/src/lib/load-plugin.ts @@ -64,11 +64,10 @@ export const loadPlugin = async function ( }, apiExtensions, ); - plugins.push(plugin); } catch (error) { closeAllPlugins(); - console.error(error); + throw error; } }; @@ -77,12 +76,12 @@ export const ɵloadPlugin = async function ( closeCallback?: () => void, apiExtensions?: object, ) { - loadPlugin(manifest, closeCallback, apiExtensions); + await loadPlugin(manifest, closeCallback, apiExtensions); }; export const ɵloadPluginByUrl = async function (manifestUrl: string) { const manifest = await loadManifest(manifestUrl); - ɵloadPlugin(manifest); + await ɵloadPlugin(manifest); }; export const ɵunloadPlugin = function (id: Manifest['pluginId']) { diff --git a/plugins/libs/plugins-runtime/src/lib/plugin-manager.ts b/plugins/libs/plugins-runtime/src/lib/plugin-manager.ts index 5b0b1bddd0..0b4035794a 100644 --- a/plugins/libs/plugins-runtime/src/lib/plugin-manager.ts +++ b/plugins/libs/plugins-runtime/src/lib/plugin-manager.ts @@ -21,6 +21,7 @@ export async function createPluginManager( let modal: PluginModalElement | null = null; let uiMessagesCallbacks: ((message: unknown) => void)[] = []; const timeouts = new Set>(); + const intervals = new Set>(); const allowDownloads = !!manifest.permissions.find( (s) => s === 'allow:downloads', @@ -55,6 +56,9 @@ export async function createPluginManager( timeouts.forEach(clearTimeout); timeouts.clear(); + intervals.forEach(clearInterval); + intervals.clear(); + if (modal) { modal.removeEventListener('close', closePlugin); modal.remove(); @@ -151,6 +155,9 @@ export async function createPluginManager( get timeouts() { return timeouts; }, + get intervals() { + return intervals; + }, get code() { return code; },