🐛 Surface real plugin errors instead of bare "Value not valid"

The plugin proxy `:on-error` handler funneled every throwable through
`handle-error`, which only produced a message for malli ExceptionInfo
carrying `::sm/explain`. Two paths lost the real cause and rendered the
unhelpful "[PENPOT PLUGIN] Value not valid. Code: :error":

- A plain JS error (TypeError, etc.) is not an ExceptionInfo, so
  `(ex-data cause)` is nil and the message bound to nil; the real
  `.-message` only reached the host-page console, invisible to the
  plugin sandbox.
- A malli explain whose errors flatten to nothing made `error-messages`
  return "", which rendered as "Value not valid: . Code: :error".

`error-messages` now returns nil (not "") when there is nothing to
render, and `handle-error` falls back to the raw explain, then to the
throwable's `ex-message`/`str`, so a useful message always surfaces.
Working cases (well-formed explain) are unchanged.

Adds regression tests for the plain-JS-error path, the empty-explain
path, and the `error-messages` nil contract.

Fixes #9692

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Filip Sajdak <filip.sajdak@siili.com>
This commit is contained in:
Filip Sajdak 2026-06-17 13:23:26 +02:00 committed by Alonso Torres
parent f1f5a30624
commit 8fe835a9cc
2 changed files with 45 additions and 8 deletions

View File

@ -311,12 +311,16 @@
(defn error-messages
[explain]
(->> (:errors explain)
(reduce csm/interpret-schema-problem {})
(flatten-error-map)
(map (fn [[field message]]
(tr "plugins.validation.message" field message)))
(str/join ". ")))
(let [msg (->> (:errors explain)
(reduce csm/interpret-schema-problem {})
(flatten-error-map)
(map (fn [[field message]]
(tr "plugins.validation.message" field message)))
(str/join ". "))]
;; Return nil (not "") when the explain has no mappable errors, so
;; `handle-error` can fall back to a non-empty message instead of
;; surfacing a bare "Value not valid. Code: :error" (#9692).
(when-not (str/blank? msg) msg)))
(defn handle-error
"Function to be used in plugin proxies methods to handle errors and print a readable
@ -340,8 +344,8 @@
(if explain
(do
(js/console.error (sm/humanize-explain explain))
(error-messages explain))
(ex-data cause))]
(or (error-messages explain) (pr-str explain)))
(or (ex-data cause) (ex-message cause) (str cause)))]
(js/console.log (.-stack cause))
(not-valid plugin-id :error message))))))

View File

@ -54,3 +54,36 @@
;; No validation errors -> no output (callers join with ". " and would
;; otherwise emit an empty string, which is fine).
(t/is (empty? (flatten-error-map {}))))
;; ---------------------------------------------------------------------
;; Issue #9692 — `handle-error` must surface a useful message instead of a
;; bare "Value not valid. Code: :error".
;;
;; `not-valid` is redefined to capture the rendered message directly, so the
;; assertions don't depend on `st/state` or the console.
(t/deftest test-handle-error-plain-js-error
;; A plain JS error has no `::sm/explain` and CLJS `ex-data` returns nil, so
;; the handler must fall back to the error's own message rather than nil.
(let [captured (atom ::none)]
(with-redefs [plugins.utils/not-valid (fn [_plugin-id _code value]
(reset! captured value) nil)]
((plugins.utils/handle-error #uuid "00000000-0000-0000-0000-000000000000")
(js/Error. "boom: not a function")))
(t/is (= "boom: not a function" @captured))))
(t/deftest test-handle-error-empty-explain
;; An explain whose errors don't render any message must not produce an
;; empty string; the handler falls back to the raw explain.
(let [captured (atom ::none)
cause (ex-info "invalid" {:app.common.schema/explain {:errors [] :value 1}})]
(with-redefs [plugins.utils/not-valid (fn [_plugin-id _code value]
(reset! captured value) nil)]
((plugins.utils/handle-error #uuid "00000000-0000-0000-0000-000000000000") cause))
(t/is (string? @captured))
(t/is (not= "" @captured))))
(t/deftest test-error-messages-empty-returns-nil
;; `error-messages` returns nil (not "") on an explain with no mappable
;; errors, so `handle-error` can distinguish "no message" from a real one.
(t/is (nil? (plugins.utils/error-messages {:errors []}))))