Analyzer crashes on specter's caching implementation (analyzing forms that create new defs)
Closed this issue · 7 comments
I created a full reproducer here: https://github.com/latacora/clerk-analyzer-specter-bug
The bug seems to be caused because Specter has a macro expansion that itself new vars (for local cache purposes).
For search purposes, the error looks like this:
Unhandled clojure.lang.ExceptionInfo
Hash is missing on dependency 'whatever/pathcache1234' of the form '(defn f [x] (sr/select [...])) 'in foo.bar.baz
I wonder if the analyzer could just compute the hash when this happens?
As a workaround, use select*
, which is the version with no fancy caching.
FYI: redplanetlabs/specter#194
I don't think this works for clerk's use case: clerk analyzes tons of stuff marked :private
, but at least it means specter recognizes it might do things to cooperate with tooling that isn't expecting it to do any of that.
(Also for me, the select
call is in an external library, and I don't think specter allows controlling cache behavior globally, so "replace with the starred variant" isn't really much of an option.
@lvh thanks for the detailed repro. I think we should at least support turning off the error via {:nextjournal.clerk/error-on-missing-vars :off}
. Clerk's error display in this case is also far from good.
Yep, that makes sense. Do we know of any cases where erroring hard is the correct behavior? I feel like warnings are better, because IIUC the worst case when you hit this is that Clerk doesn't re-render correctly?
Since this is dynamically generated I'm not even sure you could compute this hash if you wanted to. That's kind of the point of this error: it's telling the truth, the analyzer can't get to the source code.
Clerk's error display in this case is also far from good.
I think there's a regression due to #629 which prevents to reach the intended exception with a more understandable message:
clerk/src/nextjournal/clerk/analyzer.clj
Lines 334 to 335 in 3acd9cf
the hash missing check should not throw, maybe just warn
clerk/src/nextjournal/clerk/analyzer.clj
Lines 631 to 632 in 3acd9cf
I also bumped into this error when debugging stuff in clerk itself, and changing in nextjournal.clerk.view
:
(defn doc->viewer
([doc] (doc->viewer {} doc))
([opts {:as doc :keys [ns file]}]
(def d doc) ;; NOTE THIS LINE
(binding [*ns* ns]
(-> (merge doc opts) v/notebook v/present))))
I can't repro it in another namespace though
@borkdude thanks for the repro. This helps understand some of the edge cases which can trigger the exception. In this case
Hash is missing on dependency 'nextjournal.clerk.cljs-libs/prepend-required-cljs' of the form '(defn doc->viewer ([doc] (doc->viewer {} doc)) ([opts {:as doc, :keys [ns file]}] (def d :doo) (binding [*ns* ns] (-> (merge doc opts) v/notebook v/present cljs-libs/prepend-required-cljs))))' in ahoi (id: nextjournal.clerk.view/anon-expr-5dsZdRsdGo2tMPyWFe9fq2nNXNVq85, key: nextjournal.clerk.view/doc->viewer)
In the specific case the form (defn doc->viewer ...
is stored in the analyzer graph with multiple different keys (since it defines more than one var) not all of which are related with edges to the shared dependency nextjournal.clerk.cljs-libs/prepend-required-cljs
and hence can occur in random reciprocal order in the sorted keys.
Here only nextjournal.clerk.view/anon-expr-5dsZdRsdGo2tMPyWFe9fq2nNXNVq85
(the id of the code block) depends on nextjournal.clerk.cljs-libs/prepend-required-cljs
but not nextjournal.clerk.view/doc->viewer
. This is something we might want to fix.