Exception when using with Spec
Closed this issue · 13 comments
Just thought i'd give Sayid a whirl as it really looks promising, I to want to stop 'sprinkling'.
Configured everything and got this little monster:
kixi.datastore.schema-creator/schema-validator schema_creator.clj: 18
clojure.spec/valid? spec.clj: 744
clojure.spec/map-spec-impl/reify/conform* spec.clj: 782
clojure.spec/conform spec.clj: 150
clojure.spec/or-spec-impl/reify/conform* spec.clj: 1041
clojure.spec/or-spec-impl/fn spec.clj: 1006
clojure.spec/map-spec-impl/reify/conform* spec.clj: 782
clojure.spec/conform spec.clj: 150
clojure.spec/regex-spec-impl/reify/conform* spec.clj: 1633
clojure.spec/re-conform spec.clj: 1589
clojure.spec/deriv spec.clj: 1463 (repeats 3 times)
clojure.spec/deriv spec.clj: 1456
clojure.spec/dt spec.clj: 727
clojure.spec/dt spec.clj: 731
clojure.spec/conform spec.clj: 150
clojure.spec/multi-spec-impl/reify/conform* spec.clj: 900
clojure.spec/multi-spec-impl/predx spec.clj: 886
java.lang.ClassCastException: com.billpiel.sayid.trace$shallow_tracer$tracing_wrapper__22432 cannot be cast to clojure.lang.MultiFn`
Looks like spec doesn't like some of the fn wrapping that's going on. Hopefully will get the chance to dig into this a bit tomorrow, but thought I'd see if this was a common problem.
@mantree First report of this. I just started using spec today, so maybe I'll run into it too.
Were you using spec's instrumentation feature when this happened? Any other details?
thanks
It's coming from here:
Which is a direct spec/valid? call. To be sure, I disabled my instrumentation and it threw the same exception.
There is a multi-spec in the spec that is being validated with. Spec is expecting to see a multi-method in there and exploding when the wrapped function isn't one:
predx #(let [^clojure.lang.MultiFn mm @mmvar]
(c/and (.getMethod mm ((.dispatchFn mm) %))
(mm %)))
So, I reckon what's happening is Sayid is wrapping the multi-method defined here:
in something that isn't a clojure.lang.MultiFn.
I'm having a bit of trouble selectively tracing namespaces at the minute, I'll have a go tomorrow and see if it works better.
@mantree Agreed. s/multi-spec is telling spec to expect a multi-method.
Even before this, I was hoping to improve sayid's handling of multi-methods somehow. Maybe it would be better if it wrapped the defmethod fns but not defmulti fn. Not sure if that's right, or even how to do that, at the moment.
For now, as you said, avoid tracing that ns or function with sayid.
Any suggestions or input appreciated.
Ideally, the dispatch function would get wrapped, a nice source of bugs those! Could be a bit tricky to ensure a new MultiFn wrapper works nicely. Could try mutating the current one, it's marked as final though, so that could cause some interesting problems! Maybe best to just ignore them for abit.
Getting the defmethod's wrapped seems like a must though. Does that not currently work?
@mantree Sayid wraps the resulting MultFn, which covers the calls to the defmethods, but the defmethods aren't wrapped individually -- if that make sense.
After looking over MultiFn.java and playing around a bit, I'm optimistic these improvements are doable.
After a bit of selective tracing I just had a very nice experience with Sayid pointing me to where the real problem was, rather than where I thought it was. No sprinkling required!
Looks like you've made a good start on the multi-fn stuff. I wonder if treating the defmethod's separately would give a better experience, allowing people to decern between the implementations sounds like it would be beneficial. Not seen what it currently looks like yet though :)
@mantree Glad it worked out eventually.
Having sayid wrap the var produced by defmulti plays nicely with the rest of the sayid code, and I believe the approach I started the other night could support a good user experience EXCEPT in the case where a user re-evals a defmethod that attaches to a defmulti that is currently traced. In that scenario, the defmethod would apply its function to sayid's traced version of the multifn, instead of the original multifn. That new defmethod function would never get called because sayid would be looking at the original multifn.
I don't know if that makes sense, but I wanted leave some note about the status of this for you (and anyone else who might be interested).
I've been focused on rewriting the inner trace feature, which seems to be nearing completion. After that, I plan to come back and redo multifn tracing.
I just released new multimethod tracing in 0.0.11. Let me know what happens if you try it out.
The "proxy object style" idea had occurred to me, but I was scared of it because it meant new, unknown depths of java interop. But, it turned out to be the simpler solution.
I believe this is fixed -- please re-open if anyone finds otherwise.
Thanks bill, I've been distracted by infrastructure woes. When I get back to code I'll be trying Sayid again for sure!