search

cognitect-labs / aws-api

AWS, data driven
Apache License 2.0
730 stars 100 forks source link

http client fails with ring/ring-jetty-adapter 1.11.0 #250

Open dharrigan opened 10 months ago

dharrigan commented 10 months ago

Thank you for your interest in helping to improve Cognitect's aws-api!

Dependencies

Clojure CLI version 1.11.1.1435

openjdk 21.0.1 2023-10-17 LTS
OpenJDK Runtime Environment Temurin-21.0.1+12 (build 21.0.1+12-LTS)
OpenJDK 64-Bit Server VM Temurin-21.0.1+12 (build 21.0.1+12-LTS, mixed mode, sharing)
ring/ring-jetty-adapter {:mvn/version "1.11.0"}
com.cognitect.aws/api {:mvn/version "0.8.686"}
com.cognitect.aws/cloudfront {:mvn/version "847.2.1365.0"}
com.cognitect.aws/endpoints {:mvn/version "1.1.12.626"}
com.cognitect.aws/s3 {:mvn/version "848.2.1413.0"}

Description with failing test case

As ring is now available as a 1.11.0 release (along with the dependency ring/ring-jetty-apapter 1.11.0), when upgrading deps and then starting the http-client (as a dependency injected resource), the following occurs:

user=> (dev/go)
Syntax error (NoSuchFieldException) compiling . at (cognitect/http_client.clj:41:9).
getProperties
user=>

Reverting back to ring/ring-jetty-adapter {:mvn/version "1.10.0"} fixes the problem.

Thank you.

-=david=-

CambodianCoder commented 10 months ago

Encountering this issue, too. Upgrading to ring-jetty-adapter 1.11.0 causes the exception

Syntax error (NoSuchFieldException) compiling . at (cognitect/http_client.clj:41:9) getProperties

A revert back to ring-jetty-adapter 1.10.0 resolves it. The motivation for the ring-jetty-upgrade was a detection from one of our security scanners for a CVE (which doesn't impact us; we're just aiming to resolve the alert).

scottbale commented 10 months ago

Thanks, and sorry about this frustration. I'm probably stating what you already realize, but

Solving the problem of aws-api's transitive dependency on jetty 9.4.x is our top priority for our next release.

ovistoica commented 9 months ago

Any idea when the transitive dependency will be solved? Currently we cannot use aws-api as we transitioned to https://github.com/sunng87/ring-jetty9-adapter which uses Jetty 12.0.x and we cannot downgrade or switch back to ring adapter as we rely on functionality from Jetty 12.0

I saw there was an effort to move the HTTP client to the JDK version

As an aside, Is there any example code of how to use the :http-client prop so we don't rely on the default-http-client?. I see it needs to implement the HttpClient protocol

(defprotocol HttpClient
  (-submit [_ request channel]
    "Submit an http request, channel will be filled with response. Returns ch.

     Request map:

     :scheme                 :http or :https
     :server-name            string
     :server-port            integer
     :uri                    string
     :query-string           string, optional
     :request-method         :get/:post/:put/:head/:delete
     :headers                map from downcased string to string
     :body                   ByteBuffer, optional
     :timeout-msec           opt, total request send/receive timeout
     :meta                   opt, data to be added to the response map

     content-type must be specified in the headers map
     content-length is derived from the ByteBuffer passed to body

     Response map:

     :status            integer HTTP status code
     :body              ByteBuffer, optional
     :headers           map from downcased string to string
     :meta              opt, data from the request

     On error, response map is per cognitect.anomalies.

     Alpha. This will absolutely change.")
  (-stop [_] "Stops the client, releasing resources"))
tobias commented 6 months ago

I have a fork of com.cognitect/http-client that uses Jetty 11: https://github.com/tobias/cognitect-http-client. I'm currently using it in production for clojars.org without issue. Folks are free to use that, and @scottbale you are welcome to use the changes there if you do decide to upgrade the official client to Jetty 11.

bhurlow commented 3 months ago

+1 on this, we can't use aws-api with https://github.com/hyperfiddle/electric since it requires Jetty 11 for websocket support

tobias commented 3 months ago

@bhurlow Does my fork work for you?

reify-kurt-wheeler commented 2 months ago

Solving the problem of aws-api's transitive dependency on jetty 9.4.x is our top priority for our next release.

Hello, it's been about 7 months since this comment. Is there any solution for this issue yet?

terjesb commented 2 months ago

Hello, it's been about 7 months since this comment. Is there any solution for this issue yet?

(In the mean time, for us Tobias' fork worked fine on Jetty 11. Jetty 11 reached end of community support as of January 2024. Per https://github.com/cognitect-labs/aws-api/issues/181#issuecomment-2293312773, we forked Tobias' Jetty 11 fork and made it work with Jetty 12.)

scottbale commented 4 weeks ago

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.

@dharrigan 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.)