ghostwheel icon indicating copy to clipboard operation
ghostwheel copied to clipboard

Macro idea for async instrumentation: >defn-go

Open tslocke opened this issue 6 years ago • 6 comments

I have a lot of core.async in my (CLJS) app. I guess spec + core.async is a very big topic, and at some point the Clojure community will probably come up with a powerful approach. In the mean time, this is more of a low-hanging-fuit idea that could be very helpful.

In async code there are a lot of functions where the top level form is (go ...). Often the return value is discarded, but sometimes not, e.g. an async function to fetch a value from a server via HTTP.

The normal return spec is pretty much useless, as such functions always return a channel.

What you really want is to be able to spec the (single) value that is sent over the returned channel. From an instrumentation point of view, this could be done if we lift the (go ...) to be part of the macro that defines the function (and the specs), i.e. something like this:

(>defn-go my-fn 
  [...args]
  [...arg-specs => ret-spec]
  ...body)

which, when instrumented, would expand to something along the lines of

(defn my-fn
  [...args]
  ; check passed args against arg-specs (as usual)
  (go
    (let [result# ...body]
      ; check result# against ret-spec
      result#)))

I'm intending on having a crack at implementing this macro myself. Any advice would be greatly appreciated. If it sounds like something that could be a contribution, so much the better.

tslocke avatar Apr 01 '19 08:04 tslocke

This sounds interesting. A couple of quick thoughts off the top of my head:

  • At the moment Ghostwheel is only meant to be used during development and any spec-related code is stripped in production. I assume you'd want to use this in production?
  • You'd probably want to use the ^::g/outstrument metadata to enable checking on the fn input and output, but instead of simply doing spec-instrumentation with orchestra as it is usually done in Ghostwheel, you'd do regular spec-instrumentation to check the input and implement the output check as described above.
  • This sounds like it might be better to put into a separate opt-in module that depends on the main ghostwheel module, similar to https://github.com/gnl/ghostwheel.specs for example. This way I wouldn't have to change the general policy of "everything ghostwheel-related is gone in production" if I decide to add this to the project and if I don't, you could simply have a separate ghostwheel module without having to fork and maintain the whole thing.

gnl avatar Apr 03 '19 09:04 gnl

The wrapping of the body of the fn with (go ...) would have to remain in place in production, but I was thinking in terms of the normal behaviour for the instrumentation part - only have that code generated for development, and when :outstrument true is configured. It looks like I would need something like this in my macro def, to get at that config, right?

(let [{:keys [::g/outstrument]} (g/merge-config (meta fn-name))]
  ...)

I was also thinking >defn-go would be implemented in terms of >defn, so I'd only have to add the outstrument part.

Yes I'd have to disable the regular "outstrumenting" with orchestra, as the regular return value (a channel) would fail the spec. I can set that in the metadata on fn-name before calling defn>.

tslocke avatar Apr 03 '19 11:04 tslocke

I think I wanna have something like this in there—async is obviously hugely important—but I'll have to do some more thinking and a bit of exploratory programming to be able to comment on this any further and it'll probably be a while because right now I'm focused on getting 0.4.0 out the door. I'm setting the milestone to 0.5.0 for now.

gnl avatar Jul 14 '19 13:07 gnl

Your comment has prompted me to get back to this and post a simple macro I've started using. Right now it's CLJS only, and doesn't support nil in place of the spec vector. May well have other issues but I'm using it and getting some value out of it.

Other than the nil spec issue it's basically a drop-in replacement for >defn. As discussed you get an implicit (go ...) around your function body, so you mustn't also have an explicit one.

I added a bit of a hack to make failure messages more informative. Instead of emitting s/assert with a literal spec, the macro emits an (s/def ::my-ns/my-function-name-return ...). The failure message will then lead you directly to the offending function.

   (defmacro >defn-go [fn-name & fn-tail]
     (let [front-stuff (take-while (complement vector?) fn-tail)
           rest        (drop (count front-stuff) fn-tail)
           [args spec & body] rest
           spec*       (concat (drop-last spec)
                               [`any?])
           ret-spec    (last spec)
           ret-spec-kw (keyword (str ana/*cljs-ns*) (str (name fn-name) "-return"))]
       `(do
          (cljs.spec.alpha/def ~ret-spec-kw ~ret-spec)
          (ghostwheel.core/>defn ~fn-name ~@front-stuff ~args [~@spec*]
            (go (cljs.spec.alpha/assert ~ret-spec-kw (do ~@body)))))))

tslocke avatar Jul 14 '19 14:07 tslocke

👋 Hi. I'm posting this same comment to all issue threads to just give a quick heads up that the project, despite rumours and some evidence to the contrary, is not dead. It was hibernating for a little while and now nearing the long-awaited next release, which will fix some long-standing issues (and introduce some breaking changes to the config).

I'll be reviewing all open issues and PRs over the next couple of weeks, so stay tuned and thanks for the patience.

See also this Slack thread

gnl avatar May 21 '20 01:05 gnl