Make v2 plugins default throw on error (#10433)

This commit is contained in:
Alonso Torres 2026-06-30 14:50:50 +02:00 committed by GitHub
parent e0be9f7ade
commit 8823f7ac4d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 89 additions and 26 deletions

View File

@ -30,6 +30,7 @@
(def schema:registry-entry
[:map
[:plugin-id :string]
[:version {:optional true} :int]
[:name :string]
[:description {:optional true} :string]
[:host :string]

View File

@ -82,7 +82,7 @@
(defn- load-plugin!
[{:keys [plugin-id name version description host code icon permissions]}]
(st/emit! (pflag/clear plugin-id)
(st/emit! (pflag/initialize plugin-id version)
(save-current-plugin plugin-id))
(let [load-plugin (unchecked-get ug/global "ɵloadPlugin")]

View File

@ -6,17 +6,24 @@
(ns app.plugins.flags
(:require
[app.common.data :as d]
[app.main.store :as st]
[app.plugins.utils :as u]
[app.util.object :as obj]
[potok.v2.core :as ptk]))
(defn clear
[id]
(ptk/reify ::reset
(defn initialize
"Initialize flags values for plugins"
[id version]
(ptk/reify ::initialize
ptk/UpdateEvent
(update [_ state]
(update-in state [:plugins :flags] assoc id {}))))
(let [version (d/nilv version 1)]
(update-in state [:plugins :flags] assoc id
{:natural-child-ordering false
;; For version >= 2 harden the contract by throwing errors
;; on validation failures
:throw-validation-errors (>= version 2)})))))
(defn- set-flag
[id key value]

View File

@ -323,14 +323,27 @@
message to the console."
[plugin-id]
(fn [cause]
(let [message
(if-let [explain (-> cause ex-data ::sm/explain)]
(do
(js/console.error (sm/humanize-explain explain))
(error-messages explain))
(ex-data cause))]
(js/console.log (.-stack cause))
(not-valid plugin-id :error message))))
(let [explain (-> cause ex-data ::sm/explain)
throw? (throw-validation-errors? plugin-id)]
(cond
;; If it's a clojure error we throw as a validation error
(and throw? explain)
(throw-not-valid :error (error-messages explain))
;; Unexpected errors we just propagate them
throw?
(throw cause)
;; If not throw is active we log the caught error
:else
(let [message
(if explain
(do
(js/console.error (sm/humanize-explain explain))
(error-messages explain))
(ex-data cause))]
(js/console.log (.-stack cause))
(not-valid plugin-id :error message))))))
(defn is-main-component-proxy?
[p]

View File

@ -14,7 +14,8 @@
[app.util.object :as obj]
[cljs.test :as t :include-macros true]
[frontend-tests.helpers.state :as ths]
[frontend-tests.helpers.wasm :as thw]))
[frontend-tests.helpers.wasm :as thw]
[potok.v2.core :as ptk]))
(t/deftest test-common-shape-properties
(thw/with-wasm-mocks*
@ -25,6 +26,7 @@
^js context (api/create-context "00000000-0000-0000-0000-000000000000")
_ (set! st/state store)
_ (ptk/emit! store #(assoc-in % [:plugins :flags "00000000-0000-0000-0000-000000000000" :throw-validation-errors] true))
^js file (. context -currentFile)
^js page (. context -currentPage)
@ -65,7 +67,7 @@
(t/is (= (.-x shape) 10))
(t/is (= (get-in @store (get-shape-path :x)) 10))
(set! (.-x shape) "fail")
(t/is (thrown? js/Error (set! (.-x shape) "fail")))
(t/is (= (.-x shape) 10))
(t/is (= (get-in @store (get-shape-path :x)) 10)))
@ -74,7 +76,7 @@
(t/is (= (.-y shape) 50))
(t/is (= (get-in @store (get-shape-path :y)) 50))
(set! (.-y shape) "fail")
(t/is (thrown? js/Error (set! (.-y shape) "fail")))
(t/is (= (.-y shape) 50))
(t/is (= (get-in @store (get-shape-path :y)) 50)))
@ -85,7 +87,7 @@
(t/is (= (get-in @store (get-shape-path :width)) 250))
(t/is (= (get-in @store (get-shape-path :height)) 300))
(.resize shape 0 0)
(t/is (thrown? js/Error (.resize shape 0 0)))
(t/is (= (.-width shape) 250))
(t/is (= (.-height shape) 300))
(t/is (= (get-in @store (get-shape-path :width)) 250))
@ -115,7 +117,7 @@
(t/is (= (get-in @store (get-shape-path :proportion-lock)) true)))
(t/testing " - constraintsHorizontal"
(set! (.-constraintsHorizontal shape) "fail")
(t/is (thrown? js/Error (set! (.-constraintsHorizontal shape) "fail")))
(t/is (not= (.-constraintsHorizontal shape) "fail"))
(t/is (not= (get-in @store (get-shape-path :constraints-h)) "fail"))
@ -124,7 +126,7 @@
(t/is (= (get-in @store (get-shape-path :constraints-h)) :right)))
(t/testing " - constraintsVertical"
(set! (.-constraintsVertical shape) "fail")
(t/is (thrown? js/Error (set! (.-constraintsVertical shape) "fail")))
(t/is (not= (.-constraintsVertical shape) "fail"))
(t/is (not= (get-in @store (get-shape-path :constraints-v)) "fail"))
@ -175,7 +177,7 @@
(t/is (= (.-blendMode shape) "multiply"))
(t/is (= (get-in @store (get-shape-path :blend-mode)) :multiply))
(set! (.-blendMode shape) "fail")
(t/is (thrown? js/Error (set! (.-blendMode shape) "fail")))
(t/is (= (.-blendMode shape) "multiply"))
(t/is (= (get-in @store (get-shape-path :blend-mode)) :multiply)))
@ -194,7 +196,7 @@
:color {:color "#fabada" :opacity 1}
:hidden false}]))))
(let [shadow #js {:style "fail"}]
(set! (.-shadows shape) #js [shadow])
(t/is (thrown? js/Error (set! (.-shadows shape) #js [shadow])))
(t/is (= (-> (. shape -shadows) (aget 0) (aget "style")) "drop-shadow"))))
(t/testing " - blur"
@ -211,7 +213,7 @@
(t/is (= (-> (. shape -exports) (aget 0) (aget "suffix")) "test"))
(t/is (= (get-in @store (get-shape-path :exports)) [{:type :pdf :scale 2 :suffix "test" :skip-children false}]))
(set! (.-exports shape) #js [#js {:type 10 :scale 2 :suffix "test"}])
(t/is (thrown? js/Error (set! (.-exports shape) #js [#js {:type 10 :scale 2 :suffix "test"}])))
(t/is (= (get-in @store (get-shape-path :exports)) [{:type :pdf :scale 2 :suffix "test" :skip-children false}])))
(t/testing " - flipX"
@ -234,7 +236,7 @@
(t/is (= (get-in @store (get-shape-path :rotation)) 0)))
(t/testing " - fills"
(set! (.-fills shape) #js [#js {:fillColor 100}])
(t/is (thrown? js/Error (set! (.-fills shape) #js [#js {:fillColor 100}])))
(t/is (= (get-in @store (get-shape-path :fills)) [{:fill-color "#B1B2B5" :fill-opacity 1}]))
(t/is (= (-> (. shape -fills) (aget 0) (aget "fillColor")) "#B1B2B5"))

View File

@ -32,6 +32,7 @@
store (ths/setup-store file)
_ (set! st/state store)
_ (set! st/stream (ptk/input-stream store))
_ (ptk/emit! store #(assoc-in % [:plugins :flags "00000000-0000-0000-0000-000000000000" :throw-validation-errors] true))
context (api/create-context "00000000-0000-0000-0000-000000000000")]
{:file file :store store :context context}))
@ -89,9 +90,11 @@
^js page2 (aget pages 1)]
(t/is (instance? js/Promise (.openPage context page2 true)))))
(t/deftest test-open-page-invalid-arg-returns-nil
(t/deftest test-open-page-invalid-arg-throws
;; With throwValidationErrors enabled an invalid argument surfaces as an
;; exception instead of being silently logged.
(let [^js context (:context (setup))]
(t/is (nil? (.openPage context "not-a-page")))))
(t/is (thrown? js/Error (.openPage context "not-a-page")))))
(t/deftest test-open-page-resolves-when-page-changes
(t/async done

View File

@ -45,6 +45,13 @@ import type { Shape } from '@penpot/plugin-types';
<button type="button" data-appearance="secondary" (click)="resizeH()">
Resize H
</button>
<button
type="button"
data-appearance="secondary"
(click)="throwValidation()"
>
Throw validation error
</button>
<button
type="button"
data-appearance="secondary"
@ -304,6 +311,10 @@ export class AppComponent {
this.#sendMessage({ content: 'resize-h', data: { id } });
}
throwValidation() {
this.#sendMessage({ content: 'throw-validation' });
}
loremIpsum() {
this.#sendMessage({ content: 'lorem-ipsum' });
}

View File

@ -24,6 +24,8 @@ penpot.ui.onMessage<{ content: string; data: unknown }>(async (message) => {
resizeW(message.data as { id: string });
} else if (message.content === 'resize-h') {
resizeH(message.data as { id: string });
} else if (message.content === 'throw-validation') {
throwValidation();
} else if (message.content === 'lorem-ipsum') {
loremIpsum();
} else if (message.content === 'add-icon') {
@ -203,6 +205,29 @@ function resizeH(data: { id: string }) {
}
}
function throwValidation() {
const rect = penpot.createRectangle();
const center = penpot.viewport.center;
rect.x = center.x;
rect.y = center.y;
// `x` expects a number. Assigning an invalid value triggers a validation
// error so the v1 vs v2 manifest behavior can be compared:
// - v1 manifest: the error is only logged to the console and the value is
// left untouched (no exception, the try block completes).
// - v2 manifest: `penpot.flags.throwValidationErrors` defaults to true, so
// the assignment throws synchronously and the catch block runs.
try {
(rect as unknown as { x: unknown }).x = 'not-a-number';
console.log(
'[validation] No exception thrown — v1 / log-only behavior. rect.x =',
rect.x,
);
} catch (err) {
console.error('[validation] Exception thrown — v2 / strict behavior:', err);
}
}
function loremIpsum() {
const selection = penpot.selection;

View File

@ -1749,7 +1749,8 @@ export interface Flags {
/**
* If `true` the validation errors will throw an exception instead of displaying an
* error in the debugger console.
* Defaults to false
* Defaults to `true` for plugins whose manifest declares `"version": 2` (or higher)
* and to `false` for v1 plugins (or manifests that omit the version field).
*/
throwValidationErrors: boolean;
}