Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Jetty client version > 9.x #181

Open
nniv-r7 opened this issue Apr 1, 2021 · 19 comments
Open

Support Jetty client version > 9.x #181

nniv-r7 opened this issue Apr 1, 2021 · 19 comments

Comments

@nniv-r7
Copy link

nniv-r7 commented Apr 1, 2021

Problem/Feature Description
There is no obvious way (at least to me) to use aws-api library with Jetty client that is later than version 9.x.

When instantiating an HTTP client, function cognitect.http-client/ssl-context-factory calls the Jetty constructor of SslContextFactory.
While this works with Jetty 9.x, in Jetty 10.x this class became abstract, so (depending on the Jetty version) the call should be to the constructor of the subclass, i.e.: (SslContextFactory$Client. false). Otherwise the client creation fails with:

Execution error (InstantiationError) at cognitect.http-client/ssl-context-factory (http_client.clj:253).
org.eclipse.jetty.util.ssl.SslContextFactory

Suggestions
One possible way to solve this in a generic manner is to extract the call to the SslContextFactory (or SslContextFactory$Client) constructor from the ssl-context-factory function, either to some optional function parameter or to a var that can be rebound by library users, as needed.

If there is a way to use the library as-is with Jetty client that is later than version 9.x., perhaps it should be documented.

Thanks.

@dchelimsky
Copy link
Contributor

There is currently no way to do this, however we are working on a "bring your own http client" feature that we hope to release later this year that should solve this for you. Stay tuned.

@seancorfield
Copy link

We just ran into this because we've switched from Jetty 9 to Jetty 11 across the board, due to 9.x being out of community support, only to find that the Cognitect AWS client breaks.

@dchelimsky
Copy link
Contributor

dchelimsky commented Nov 23, 2022

Update:

aws-api depends on the cognitect http-client, and is subject to its jetty dependency. Since that http-client has other users besides aws-api, we can't depend on it upgrading its jetty dependency on any timeline, if ever.

We have done some design work on the "bring your own http client" feature, and while I can't really give you a timeline, I can tell you the direction:

  • make the HttpClient protocol part of the public API
    • Important: today it is not part of the public API and it will most likely change!
  • publish reifications for the cognitect http-client (which will continue to be the default as not to make a breaking change), and the java.net.http.HttpClient. The java client was released in Java 11, however it comes with some constraints that we are still evaluating, so it may be that it is only supported on Java >= 17.
  • introduce a mechanism to override the default http client so that whenever you create a new aws client, it defaults to your choice

Given that plan, you could do the following right now, with the understanding that you are using undocumented APIs/features that are very likely to change before we publish them. You've been warned!


WARNING: 🚧 THE FOLLOWING IS NOT OFFICIAL AND COMES WITH ZERO PROMISES 🚧

  • add the dependencies necessary to support the http client of your choosing
    • optionally exclude com.cognitect/http-client from the com.cognitect.aws/api dependency.
  • write a function that returns a reification of the HttpClient protocol that delegates submit to the http client you want to use
    • or deftype or defrecord implementing the HttpClient protocol
    • you can use cognitect.aws.http.cognitect/create (internal fn/undocumented) as a model. The heavy lifting is done inside the cognitect http-client, which is open source but not hosted in any public git repo. To see the source, you'll need to look in the jar, which you should find in ~/.m2/repository/com/cognitect/http-client/1.0.115/http-client-1.0.115.jar if you're using aws-api locally.
  • invoke that function and pass the reified object to cognitect.aws.client.api/client e.g.
(cognitect.aws.client.api/client {:api <api> :http-client (make-my-client)})

For now, you'll have do that for every aws-client you create, but we plan to make it so you can register a default when we release this feature.

WARNING: 🚧 THE PRECEDING IS NOT OFFICIAL AND COMES WITH ZERO PROMISES 🚧


If anybody tries this, please report back on your success! Once we release this, we'll be looking for others in the community to release wrappers for e.g. jetty >= 10 and any other http clients you wish to use.

@grzm
Copy link

grzm commented Nov 24, 2022

FWIW, I replaced the cognitect http-client with a JDK 11 java.net.http version for awyeah-api:

https://github.com/grzm/awyeah-api/blob/main/src/com/grzm/awyeah/http_client.clj

Currently it has one known infelicity: I'm sure there are others. It's purpose was to mimic as faithfully as possible the behavior of the cognitect http-client to limit surprises. It wouldn't surprise me if this isn't the best way to construct such a client.

@dchelimsky
Copy link
Contributor

@grzm s/congitect.http-client/cognitect.http-client ;)

Also, thanks for posting!

@grzm
Copy link

grzm commented Nov 24, 2022

LOL. The number of times I've made that typo is embarrassing :)

@dchelimsky
Copy link
Contributor

Don't tell anybody, but I'll bet I've mistyped chelimsky more than you've mistyped cognitect ;)

@xiongtx
Copy link

xiongtx commented Mar 9, 2023

Here's what I had to do in order to use an HTTP proxy successfully.

Start by looking at the source code for cognitect.http-client/create. Since it's is not available on GitHub, only way to do so is through the JAR. Below is for version 1.0.115:

(defn create
  "Creates an http-client that can be used with submit. Takes a config map with
   the following keys:

   :resolve-timeout                  in msec, default 5000
   :connect-timeout                  in msec, default 5000
   :max-connections-per-destination  default 64
   :pending-ops-limit                default 64
   :classpath-trust-store            classpath location of trust store
   :trust-store-password             trust store password
   :trust-store                      java.security.KeyStore instance
   :validate-hostnames               boolean, defaults to true
   :proxy-host                       optional host to use for proxy, string, defaults to 'localhost' if proxy-port is provided with no proxy-host
   :proxy-port                       optional port to use for proxy, int"
  [{:keys [resolve-timeout connect-timeout max-connections-per-destination
           pending-ops-limit proxy-host proxy-port]
    :or   {resolve-timeout 5000
           connect-timeout 5000
           max-connections-per-destination 64
           pending-ops-limit 64}
    :as   config}]
  (configure-jetty-announce)
  (let [jetty-client (doto (HttpClient. (ssl-context-factory config))
                       (.setAddressResolutionTimeout resolve-timeout)
                       (.setConnectTimeout connect-timeout)
                       (.setMaxConnectionsPerDestination max-connections-per-destination)
                       ;; these are Jetty defaults, except for the queue size
                       (.setExecutor (doto (org.eclipse.jetty.util.thread.QueuedThreadPool.
                                            200 8 60000
                                            (java.util.concurrent.LinkedBlockingQueue. ^int (* 2 pending-ops-limit)))
                                       (.setDaemon true))))]
    ;; This is jetty defaults, except daemonizing scheduler
    (.setScheduler jetty-client (org.eclipse.jetty.util.thread.ScheduledExecutorScheduler.
                                  (str (-> jetty-client class .getSimpleName) "@" (.hashCode jetty-client) "-scheduler") true))
    (when proxy-port
      (-> jetty-client
          .getProxyConfiguration
          .getProxies
          (.add (Socks4Proxy.
                  (or ^String proxy-host
                      "localhost")
                  ^int proxy-port))))
    (.start jetty-client)
    (Client.
      jetty-client
      (atom 0)
      pending-ops-limit)))

Note that it takes :proxy-host/:proxy-port as parameters. But I needed an HttpProxy, not Socks4Proxy. So I modified that part.

(-> jetty-client
    .getProxyConfiguration
    .getProxies
    (.add (HttpProxy. ^String proxy-host ^int proxy-port)))

Then make a copy of cognitect.aws.http.cognitect/create using your custom create:

(defn custom-http-client
  []
  (let [c (-> (if (and proxy-host proxy-port)
                {:proxy-host proxy-host
                 :proxy-port proxy-port}
                {})
              create)]
    (reify aws/HttpClient
      (-submit [_ request channel]
        (impl/submit c request channel))
      (-stop [_]
        (impl/stop c)))))

And set the :http-client for the AWS clients to (custom-http-client). E.g.

(def ssm-client
  (aws/client {:api :ssm
               :credentials-provider (cognitect-process-creds-provider)
               :http-client (custom-http-client)
               :region "us-west-2"}))

@dchelimsky
Copy link
Contributor

Thank you so much for posting this!

@xiongtx
Copy link

xiongtx commented Mar 10, 2023

Oh, and you'll want to create one instance of the HTTP client for multiple AWS clients to share, like the library does. E.g.

(def http-client (custom-http-client))

(def ssm-client
  (aws/client {:api :ssm
               :credentials-provider (cognitect-process-creds-provider)
               :http-client http-client
               :region "us-west-2"}))

(defn ec2-client
  [account-id region]
  (aws/client {:api :ec2
               :credentials-provider (cognitect-ssm-creds-provider account-id)
               :http-client http-client
               :region region}))

Creating a unique HTTP client per AWS client is a quick way to create lots more threads than you intended and give yourself an OOM error 🙃.

@skynet-gh
Copy link

I recently updated to Jetty 11 using the guides @xiongtx posted above, with the following modifications:

deps.edn:

{:deps
 {com.cognitect.aws/api {:mvn/version "0.8.686"}
  com.cognitect/http-client {:mvn/version "1.0.111"}
  org.eclipse.jetty/jetty-client {:mvn/version "11.0.15"}
  org.eclipse.jetty/jetty-http {:mvn/version "11.0.15"}
  org.eclipse.jetty/jetty-util {:mvn/version "11.0.15"}}}

com.cognitect/http-client version 1.0.115+ does not compile with Jetty 11, due to this code in configure-jetty-announce (cognitect/http_client.clj):

41  (doto (Log/getProperties)

I guess org.eclipse.jetty.util.log.Log/getProperties has been changed/removed. So use the older http-client, and remove the (configure-jetty-announce) call from the defn create above.

The other issue is org.eclipse.jetty.util.ssl.SslContextFactory was changed, split into .Client and .Server subclasses. I didn't need to customize it, so I replaced the (HttpClient. (ssl-context-factory config)) with just (HttpClient.).

And then it seems to work.

@gpind
Copy link

gpind commented Jan 2, 2024

@dchelimsky has there been any progress on this since your last update?

@dchelimsky
Copy link
Contributor

Hey @gpind. I'm no longer involved with this project. I think @scottbale might be able to answer your question.

@scottbale
Copy link
Collaborator

Hi @gpind (and @dchelimsky).

Removing the Jetty 9.x dependency is our most immediate goal.

Nothing has been released yet. But we have been working on a JDK 11-based http client which has no dependency on Jetty. Potentially it will entirely replace the cognitect http client, thus removing the Jetty 9.x transitive dependency.

We are a small team and things have slowed down in December, but I expect them to pick up again starting this month.

@gpind
Copy link

gpind commented Jan 2, 2024

@scottbale got it, and thank you for the update!

@namenu
Copy link

namenu commented Jan 3, 2024

@gpind FYI, we vendored cognitect.http-client to workaround this issue as commented above.

main...green-labs:aws-api:http-client

in order to update ring-1.11.0, I couldn't put this off any longer.

So far so good without any issues with this fix.

@tobias
Copy link

tobias commented May 6, 2024

I have a fork of com.cognitect/http-client that uses Jetty 11: https://github.com/tobias/cognitect-http-client. It is published to Clojars, and I'm currently using it in production for Clojars itself without issue.

@terjesb
Copy link

terjesb commented Aug 16, 2024

Thanks, @tobias! I have a fork of com.cognitect/http-client that adopts Jetty 12 on top of your changes for Jetty 11 support: https://github.com/terjesb/cognitect-http-client/tree/jetty-12. (Our project also uses Pedestal with the jetty-12-ee10 branch from https://github.com/terjesb/pedestal/tree/jetty-12-ee10.)

@scottbale
Copy link
Collaborator

We have just released a beta release which introduces a Java-native http client. With this release, Jetty is no longer by default a transitive dependency.

@nniv-r7 if time permits, could you please verify whether this release solves your issue? Thanks.

(I will leave this issue open for the time being until we verify.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests