ClassCastException using transformers

[wandersoncferreira]

Hello @miikka I am opening this issue to discuss the exception I mentioned on PR #248 . I did some further exploration around this problem and there are 2 (at least) different scenarios going on.

1. The Encoder/Decoder functions should handle their own exceptions?
2. Calls to s/unform using values that are not compliant with s/conform.

Let's follow an example.

(s/def ::my-spec
  (spec
   {:spec #(and (simple-keyword? %) (-> % name clojure.string/lower-case keyword (= %)))
    :description "a lowercase keyword, encoded in uppercase in string-mode"
    :encode/string (fn [spec value]
                     (println value) ;; => :wand or ;; => "wand"
                     (-> value name clojure.string/upper-case))}))

;; this call produces an ::s/invalid result from s/conform however, as
;; this is a leaf node the conformer let this pass through.
(encode ::my-spec :wand string-transformer)
;; => "WAND"

The :wand keyword comply with the provided spec, but the transformed value does not ("WAND").

;; this call is problematic (in some sense) because "WAND" does not
;; comply with s/conform therefore we are "stretching" s/unform a
;; little bit here. This is a gray area for me yet, what is the
;; expected output for these situations.
(s/unform ::my-spec "WAND")
;; => "WAND"

But luckly (for me) it does not produce error.

Then we can see now the two sources of problems. When we try to encode a value that produces an error inside our encoder function, we get the cryptic message:

(encode ::my-spec {:error-expected "wand"} string-transformer)

;; 1. Unhandled java.lang.ClassCastException
;;    (No message)

The error comes from here: (-> {:error-expected "wand"} name clojure.string/upper-case).

Potential solutions:

1. Improve documentation to explicitly guide users to handle their exceptions e.g. the spec-tools.transform namespace clearly handles all the possible exceptions for each transformation that can throw some error. The guideline is "if you don't know how to transform it, return as is".

2. Catch errors from transform call during the conform step and ensure the previous behavior for the user. Something like this would be enough:

;;; spec-tools.core L404
(let [transformer *transformer*, encode? *encode?*
      safe-transform (fn [t this x] (try (t this x) (catch Throwable _ x)))]
      ;; if there is a transformer present
      (if-let [transform (if transformer ((if encode? -encoder -decoder) transformer (decompose-spec-type this) x))]
        ;; let's transform it
        (let [transformed (safe-transform transform this x)]
          ...))

The other scenario is about calling s/unform with not compliant values becomes problem when this operation is not allowed. For example, the case motivating the PR #248 returns a string and when we call s/unform with the string we get another ClassCastException.

(s/def :db/hostname string?)
(s/def :db/port pos-int?)
(s/def :db/database string?)
(s/def ::jdbc-connection
  (spec {:spec (s/keys :req-un [:db/hostname :db/port :db/database])
         :type :dbconn}))

(s/unform ::jdbc-connection "jdbc:....")

I think solving this one is a lot harder, because conform* is returning values that are not valid (https://github.com/metosin/spec-tools/blob/d05e6e3c76c3c6ff847aa3f8e66344df2705aeae/src/spec_tools/core.cljc#L430) and if we remove this, examples like the first one I brought here would start breaking.

Idk, maybe wrap this to catch errors and default to doing nothing if error occur.

Would like to hear more from you about this. Thanks