redplanetlabs / specter

Clojure(Script)'s missing piece
Apache License 2.0
2.52k stars 102 forks source link

setval throwing an exception when being used in a threading macro #157

Closed imrekoszo closed 7 years ago

imrekoszo commented 7 years ago

From the slack log in #specter on clojurians: [1:37 PM]
hi all

[1:38]
I would be grateful if someone could give me advice on building paths dynamically using a navigator in 0.13.1

[1:38]
my navigator is like this

[1:38]

(sp/defnav
  EID
  [eid]
  (select* [{:keys [eid]} structure next-fn] (eid-nav-select* eid structure next-fn))
  (transform* [{:keys [eid]} structure next-fn] (eid-nav-transform* eid structure next-fn)))

[1:39]
and this is how I try to build a path from a list of keys

[1:39]

(defn- specter-path [ks]
  (map #(if (u/uuid? %)
          (EID %)
          %)
       ks))

[1:41]
and getting a Cannot statically combine sequential when not in nav pos error when invoking specter-path

imre [1:49 PM]
well actually the error comes when I try to use the path in a call to setval

imre [2:02 PM]
well, I managed to fix it however I don't exactly know why it failed.

[2:02]
so the calling code looked like this:

[2:02]

(-> (concat [:_entity] ks)
          specter-path
          (sp/setval value stored-entity))

(edited)

[2:03 PM]
but when I defined this function

(defn- setval [apath aval structure]
  (sp/setval apath aval structure))

and replaced sp/setval with setval inside the threading macro, it worked

nathanmarz [2:12 PM]
@imre that looks like a bug

[2:12]
can you open an issue on github for it?

[2:12]
you can workaround it by doing (let [p (specter-path ...)] (setval p ...))

nathanmarz commented 7 years ago

I looked into this and am unable to reproduce this.

nathanmarz commented 7 years ago

Closing due to inability to reproduce.

mkorvas commented 7 years ago

I just saw the same error with specter version 1.0.0 and clojure version 1.7.0. Although I don't see quickly how to provide a minimal example, perhaps sharing the stacktrace here would still be helpful:

#error
 {
 :cause Cannot statically combine sequential when not in nav pos
 :via
 [{:type java.lang.IllegalArgumentException
   :message Cannot statically combine sequential when not in nav pos
   :at [com.rpl.specter.impl$static_combine invoke impl.cljc 700]}]
 :trace
 [[com.rpl.specter.impl$static_combine invoke impl.cljc 700]
  [com.rpl.specter.impl$static_combine$fn__2118 invoke impl.cljc 712]
  [clojure.core$map$fn__4553 invoke core.clj 2622]
  [clojure.lang.LazySeq sval LazySeq.java 40]
  [clojure.lang.LazySeq seq LazySeq.java 49]
  [clojure.lang.RT seq RT.java 507]
  [clojure.core$seq__4128 invoke core.clj 137]
  [clojure.core$dorun invoke core.clj 3009]
  [clojure.core$doall invoke core.clj 3025]
  [com.rpl.specter.impl$static_combine invoke impl.cljc 712]
  [com.rpl.specter.impl$static_combine invoke impl.cljc 694]
  [clojure.core$map$fn__4553 invoke core.clj 2624]
  [clojure.lang.LazySeq sval LazySeq.java 40]
  [clojure.lang.LazySeq seq LazySeq.java 49]
  [clojure.lang.RT seq RT.java 507]
  [clojure.core$seq__4128 invoke core.clj 137]
  [clojure.core$dorun invoke core.clj 3009]
  [clojure.core$doall invoke core.clj 3025]
  [com.rpl.specter.impl$static_combine invoke impl.cljc 703]
  [com.rpl.specter.impl$static_combine invoke impl.cljc 694]
  [com.rpl.specter.impl$magic_precompilation invoke impl.cljc 884]
  [kea.users$set_verdict_BANG_$fn__5870$fn__5871 invoke users.clj 537]
  ...

The application code mentioned on the last line of the stacktrace updates a certain path in the HTTP session atom mem:

535      ; Update the session.
536      (run! (fn [ring-id]
537              (swap! mem #(transform (concat [(keypath ring-id) :noir :kea-user]
538                                             specter-key)
539                                     (fn [state] (merge state v-up-map))
540                                     %)))
541            ring-ids)
mkorvas commented 7 years ago

This seems to be a good minimal example:

(require '[com.rpl.specter :refer [transform]])

(let [specter-key [:state]]
  (transform (concat [] specter-key) identity {}))
mkorvas commented 7 years ago

Attached a trace of a slightly "more minimal" example. select-trace.txt

nathanmarz commented 7 years ago

Looks like this is already fixed on master. Try 1.0.1-SNAPSHOT

mkorvas commented 7 years ago

Indeed, this is not an issue with 1.0.1-SNAPSHOT. Good news!