clj-commons / manifold

A compatibility layer for event-driven abstractions
1.02k stars 106 forks source link

Fat-fingered d/chain manifests as mysterious error #154

Closed aengelberg closed 6 years ago

aengelberg commented 6 years ago

I accidentally forgot to wrap a chain callback in fn so I ended up basically chaining a deferred to a deferred. Here's the scenario:

;; the right way
(d/chain
  (do-a-thing)
  (fn [_]
    (do-another-thing)))
;; the way I did by accident
(d/chain
  (do-a-thing)
  (do-another-thing))

But rather than a type error I get back mysterious "successful" results:

user=> @(d/chain (d/success-deferred 1) (d/success-deferred 2))
nil
user=> @(d/chain (d/future 1) (d/future 2))
1

I see now that deferreds appear to intentionally implement IFn as a syntactic sugar for delivering things into it, so that explains the errors. But it would be nice if d/chain had some guards to protect against this class of mistake.

ztellman commented 6 years ago

Deferreds are functions because that's how Clojure's promises work, and deferreds are supposed to act as drop-in replacements. I'm not sure how to avoid this sort of mistake without precluding valid use cases, but I'm happy to hear any suggestions you might have.

aengelberg commented 6 years ago

Hmm. I had thought that one solution would be for d/chain to just throw an exception if the chained "functions" were specifically deferreds. But the more I think about it, one might legitimately (but not particularly wisely) use the sugared delivery mechanism as a callback in d/chain:

(let [d (d/deferred)]
  (d/chain
    (do-a-thing)
    ;; when my thing is done, deliver the result into `d`
    d)
  d)

So in that case I'm also not sure of a good way to eliminate my above mistake.

ztellman commented 6 years ago

Yeah, in those cases I'd just put a #(success! d %) in the chain, but I'm sure someone's done what you describe and I can't willfully break it. I'm going to close the issue, but I do apologize for the time you spent tracking this down.