[Proposal][Client] Consider more idiomatic bridges with JVM async paradigms

[bsless]

Currently, every code using HTTP Kit's client must check and handle the :error key in the response, including in asynchronous code.
Additionally, the return value from request does not "play nice" (explained further) with java.util.concurrent idioms

Given

(ns playground.http-kit
  (:require
   [org.httpkit.client :as client]
   [promesa.core :as p])
  (:import
   (java.util.concurrent CompletableFuture)))

This works

@(client/get "http://httpstat.us/200")

This doesn't work

(-> "http://httpstat.us/200"
    client/get
    (p/then println)
    deref)

This does work

(def request (p/promisify client/request))

(p/then (request {:method :get :url "http://httpstat.us/200"}) println)

This works, too

(defn request-cf
  [opts]
  (let [cf (CompletableFuture.)]
    (client/request opts (fn [{:keys [error] :as ret}]
                           (if error
                             (.completeExceptionally cf error)
                             (.complete cf ret))))
    cf))

(p/then (request-cf {:method :get :url "http://httpstat.us/200"}) println)

This also requires parsing errors, this would have been nicer

(defn request-3
  [opts on-success on-failure]
  (client/request opts (fn [{:keys [error] :as ret}]
                         (if error
                           (on-failure error)
                           (on-success ret)))))

Then request-cf would be implemented in a more straightforward manner

(defn request-cf3
  [opts]
  (let [cf (CompletableFuture.)]
    (request-3 opts (fn [ret] (.complete cf ret)) (fn [err] (.completeExceptionally cf err)))
    cf))

Consuming http kit client from j.u.c would be cleaner / easier if client/request returned a CompletableFuture

It would also be convenient to have a 3 arity version (like ring) which would not require parsing the return value and checking for error, makes error handling explicit, separates error handling code from business logic

That also simplifies writing generic wrappers for HTTP clients independent of their implementation details (instrumentations like metrics, middlewares, etc)

wdyt?

[ptaoussanis]

@bsless Thanks for this Ben 🙏

So just to check my understanding - there's 2 enhancements being proposed here:

Proposal 1

org.httpkit.client/request currently returns a Clojure promise, and Clojure promises don't currently implement some specific java.util.concurrent.CompletableFuture methods that'd be handy to support for http-kit client.

The proposal is to update org.httpkit.client/request so that it returns a different type (e.g. via reify) with support for both the current (promise) API and some key subset of the CompletableFuture methods.

Is that right? If so, sounds reasonable to me - and shouldn't be difficult - though I'm not familiar off-hand with the particular CompletableFuture details. Hopefully not too many methods would need to be implemented?

Proposal 2

org.httpkit.client/request doesn't currently offer any way to directly set its IResponseHandler methods, and some capability to optionally do that would be handy.

One obvious approach would be to add something like :on-success, :on-error opts to org.httpkit.client/request.

Is that the idea? If so, sounds very reasonable to me - and also shouldn't be difficult.

---

If I've got the above ~right, then both sound like nice additions with no obvious downsides - would be happy to look at a PR from you or anyone else that felt like submitting one 👍

[bsless]

That's the general thrust, but there are some subtle points (footguns?) for each that should be considered

Proposal 1 - CompletableFuture

The CompletableFuture interface is huge. It merges the Future interface which has ~5 methods and the CompletionStage interface which has over 20.
In that case, reifying Future is easy, but the entire interface isn't.
Possible alternatives:

• Just return a CompletableFuture (breaking)
• Extend CompletableFuture and implement IPending for it. No need to implement IDeref because deref handles Future, but then handling the *deadlock-guard* isn't trivial
• Just add Future to the reification, not worth it in my opinion
• Let the caller choose what returns

Proposal 2 - success/error callbacks

The problem with directly setting the response handler methods is all the special cases on the success callback which can still error, like too many redirects.
I thought about just adding an arity for [request on-success on-error] then implement the [request callback] arity using it without change of behavior.

[ptaoussanis]

Thanks for the feedback.

I'm currently focused on other topics so my own feedback'll need to be superficial for now-

Proposal 1

It merges the Future interface which has ~5 methods and the CompletionStage interface which has over 20.

Is it really necessary to implement so much for the use cases commonly applicable to http-kit client?

Have there been any relevant discussions re: Clojure's promise and CompletableFuture? What's the current consensus/plan for bridging the two? Are there well-built libraries that provide a bridge?

Possible alternatives: [...]

I'd rule out the breaking change, and would be hesitant to provide an option that changes the return type. Options like that tend to balloon tests, documentation, examples, etc. It's not out-of-the-question if there's a strong reason to go that route, but my first preference would definitely be to first rule out better alternatives.

Proposal 2

The problem with directly setting the response handler methods is all the special cases on the success callback which can still error, like too many redirects.

To clarify- I don't mean to suggest replacing the current handler methods, just injecting user callbacks into them at the appropriate place - as is done for the current undifferentiated callback support. The current under-the-covers logic should be retained, we can just diversify callback to differentiated on-success, on-error.

I thought about just adding an arity for [request on-success on-error] then implement the [request callback] arity

I'm not too concerned about the particular calling API - I believe that's unrelated to point above. We could easily support both kw and positional args- i.e. something like

(defn request ([{:keys [on-success on-error ...]]) ([opts cb]) ([opts on-success on-error])).

[bsless]

I'll be optimistic and try to extend CompletableFuture and see if manages to satisfy all requirements.

If all turns out well, I'll prepare two draft PRs

@borkdude would appreciate your thoughts here, is there anything I'm missing?

[borkdude]

Proposal 3 could maybe be to offer a third party bridge lib between http-kit client + CompletableFuture but at this point you might just want to use the built-in java.net.http library which does this or a Clojure wrapper like hato or babashka.http-client?