From a2672a598c338f691deec180199c8dc15bf2e518 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 25 Mar 2026 12:12:18 +0100 Subject: [PATCH] :bug: Fix TypeError when token error map lacks :error/fn key (#8767) * :bug: Fix TypeError when token error map lacks :error/fn key Guard against missing :error/fn in token form control resolve streams. When schema validation errors are produced they may not carry an :error/fn key; calling nil as a function caused a TypeError crash. Apply an if-let guard at all 7 affected sites across input.cljs, color_input.cljs and fonts_combobox.cljs, falling back to :message or returning the error map unchanged. * :recycle: Extract token error helpers and add unit tests Extract resolve-error-message and resolve-error-assoc-message helpers into errors.cljs, replacing the seven duplicated inline lambdas in input.cljs, color_input.cljs and fonts_combobox.cljs with named function references. Add frontend-tests.tokens.token-errors-test covering both helpers for the normal path (:error/fn present) and the fallback path (schema-validation errors that lack :error/fn). Signed-off-by: Penpot Dev --------- Signed-off-by: Penpot Dev --- .../main/data/workspace/tokens/errors.cljs | 20 +++++++ .../forms/controls/color_input.cljs | 9 ++-- .../forms/controls/fonts_combobox.cljs | 9 ++-- .../management/forms/controls/input.cljs | 12 ++--- frontend/test/frontend_tests/runner.cljs | 2 + .../tokens/token_errors_test.cljs | 53 +++++++++++++++++++ 6 files changed, 84 insertions(+), 21 deletions(-) create mode 100644 frontend/test/frontend_tests/tokens/token_errors_test.cljs diff --git a/frontend/src/app/main/data/workspace/tokens/errors.cljs b/frontend/src/app/main/data/workspace/tokens/errors.cljs index acc5650e30..30ab2e30b9 100644 --- a/frontend/src/app/main/data/workspace/tokens/errors.cljs +++ b/frontend/src/app/main/data/workspace/tokens/errors.cljs @@ -135,6 +135,26 @@ (defn has-error-code? [error-key errors] (some #(= (:error/code %) error-key) errors)) +(defn resolve-error-message + "Returns the human-readable message string for a single error map. + When the error carries an :error/fn key the function is called with + :error/value to produce the message. Falls back to :message for + errors that originate from schema-validation (which have no :error/fn)." + [error] + (if-let [f (:error/fn error)] + (f (:error/value error)) + (:message error))) + +(defn resolve-error-assoc-message + "Returns the error map with a :message key set to the resolved human- + readable string. When the error carries an :error/fn key the function + is called with :error/value; otherwise the map is returned unchanged + (it is expected to already carry a :message from schema-validation)." + [error] + (if-let [f (:error/fn error)] + (assoc error :message (f (:error/value error))) + error)) + (defn humanize-errors [errors] (->> errors (map (fn [err] diff --git a/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/color_input.cljs b/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/color_input.cljs index c9c2f0aaf0..1cd082dcbf 100644 --- a/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/color_input.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/color_input.cljs @@ -17,6 +17,7 @@ [app.main.data.style-dictionary :as sd] [app.main.data.tinycolor :as tinycolor] [app.main.data.tokenscript :as ts] + [app.main.data.workspace.tokens.errors :as wte] [app.main.data.workspace.tokens.format :as dwtf] [app.main.refs :as refs] [app.main.ui.ds.controls.input :as ds] @@ -277,9 +278,7 @@ (rx/debounce 300) (rx/mapcat (partial resolve-value tokens token token-name)) (rx/map (fn [result] - (d/update-when result :error - (fn [error] - ((:error/fn error) (:error/value error)))))) + (d/update-when result :error wte/resolve-error-message))) (rx/subs! (fn [{:keys [error value]}] (let [touched? (get-in @form [:touched input-name])] (when touched? @@ -439,9 +438,7 @@ (rx/debounce 300) (rx/mapcat (partial resolve-value tokens token token-name)) (rx/map (fn [result] - (d/update-when result :error - (fn [error] - (assoc error :message ((:error/fn error) (:error/value error))))))) + (d/update-when result :error wte/resolve-error-assoc-message))) (rx/subs! (fn [{:keys [error value]}] diff --git a/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/fonts_combobox.cljs b/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/fonts_combobox.cljs index 057419dc9c..d663e9defe 100644 --- a/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/fonts_combobox.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/fonts_combobox.cljs @@ -13,6 +13,7 @@ [app.config :as cf] [app.main.data.style-dictionary :as sd] [app.main.data.tokenscript :as ts] + [app.main.data.workspace.tokens.errors :as wte] [app.main.fonts :as fonts] [app.main.ui.ds.buttons.icon-button :refer [icon-button*]] [app.main.ui.ds.controls.input :refer [input*]] @@ -168,9 +169,7 @@ (rx/debounce 300) (rx/mapcat (partial resolve-value tokens token token-name)) (rx/map (fn [result] - (d/update-when result :error - (fn [error] - ((:error/fn error) (:error/value error)))))) + (d/update-when result :error wte/resolve-error-message))) (rx/subs! (fn [{:keys [error value]}] (when touched? (if error @@ -291,9 +290,7 @@ (rx/debounce 300) (rx/mapcat (partial resolve-value tokens token token-name)) (rx/map (fn [result] - (d/update-when result :error - (fn [error] - ((:error/fn error) (:error/value error)))))) + (d/update-when result :error wte/resolve-error-message))) (rx/subs! (fn [{:keys [error value]}] (cond diff --git a/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/input.cljs b/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/input.cljs index 6ab3f2786b..3e20c5a2e1 100644 --- a/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/input.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/input.cljs @@ -241,9 +241,7 @@ (rx/debounce 300) (rx/mapcat (partial resolve-value tokens token token-name)) (rx/map (fn [result] - (d/update-when result :error - (fn [error] - ((:error/fn error) (:error/value error)))))) + (d/update-when result :error wte/resolve-error-message))) (rx/subs! (fn [{:keys [error value]}] (let [touched? (get-in @form [:touched input-name])] (when touched? @@ -333,9 +331,7 @@ (rx/debounce 300) (rx/mapcat (partial resolve-value tokens token token-name)) (rx/map (fn [result] - (d/update-when result :error - (fn [error] - (assoc error :message ((:error/fn error) (:error/value error))))))) + (d/update-when result :error wte/resolve-error-assoc-message))) (rx/subs! (fn [{:keys [error value]}] @@ -445,9 +441,7 @@ (rx/debounce 300) (rx/mapcat (partial resolve-value tokens token token-name)) (rx/map (fn [result] - (d/update-when result :error - (fn [error] - (assoc error :message ((:error/fn error) (:error/value error))))))) + (d/update-when result :error wte/resolve-error-assoc-message))) (rx/subs! (fn [{:keys [error value]}] diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index fe5f5efdac..ad84056110 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -17,6 +17,7 @@ [frontend-tests.tokens.logic.token-data-test] [frontend-tests.tokens.logic.token-remapping-test] [frontend-tests.tokens.style-dictionary-test] + [frontend-tests.tokens.token-errors-test] [frontend-tests.tokens.workspace-tokens-remap-test] [frontend-tests.util-object-test] [frontend-tests.util-range-tree-test] @@ -49,6 +50,7 @@ 'frontend-tests.tokens.logic.token-data-test 'frontend-tests.tokens.logic.token-remapping-test 'frontend-tests.tokens.style-dictionary-test + 'frontend-tests.tokens.token-errors-test 'frontend-tests.util-object-test 'frontend-tests.util-range-tree-test 'frontend-tests.util-simple-math-test diff --git a/frontend/test/frontend_tests/tokens/token_errors_test.cljs b/frontend/test/frontend_tests/tokens/token_errors_test.cljs new file mode 100644 index 0000000000..ae231fbe1e --- /dev/null +++ b/frontend/test/frontend_tests/tokens/token_errors_test.cljs @@ -0,0 +1,53 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC + +(ns frontend-tests.tokens.token-errors-test + (:require + [app.main.data.workspace.tokens.errors :as wte] + [cljs.test :as t :include-macros true])) + +;; --------------------------------------------------------------------------- +;; resolve-error-message +;; --------------------------------------------------------------------------- + +(t/deftest resolve-error-message-with-error-fn + (t/testing "calls :error/fn with :error/value when both keys are present" + (let [error {:error/fn (fn [v] (str "bad value: " v)) + :error/value "abc"}] + (t/is (= "bad value: abc" (wte/resolve-error-message error)))))) + +(t/deftest resolve-error-message-without-error-fn + (t/testing "returns :message when :error/fn is absent (schema-validation error)" + (let [error {:message "This field is required"}] + (t/is (= "This field is required" (wte/resolve-error-message error)))))) + +(t/deftest resolve-error-message-nil-error-fn + (t/testing "returns :message when :error/fn is explicitly nil" + (let [error {:error/fn nil :message "fallback message"}] + (t/is (= "fallback message" (wte/resolve-error-message error)))))) + +;; --------------------------------------------------------------------------- +;; resolve-error-assoc-message +;; --------------------------------------------------------------------------- + +(t/deftest resolve-error-assoc-message-with-error-fn + (t/testing "assocs :message produced by :error/fn into the error map" + (let [error {:error/fn (fn [v] (str "invalid: " v)) + :error/value "42" + :error/code :error.token/invalid-color}] + (let [result (wte/resolve-error-assoc-message error)] + (t/is (= "invalid: 42" (:message result))) + (t/is (= :error.token/invalid-color (:error/code result))))))) + +(t/deftest resolve-error-assoc-message-without-error-fn + (t/testing "returns the error map unchanged when :error/fn is absent" + (let [error {:message "This field is required"}] + (t/is (= error (wte/resolve-error-assoc-message error)))))) + +(t/deftest resolve-error-assoc-message-nil-error-fn + (t/testing "returns the error map unchanged when :error/fn is explicitly nil" + (let [error {:error/fn nil :message "fallback"}] + (t/is (= error (wte/resolve-error-assoc-message error))))))