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

Update to Vert.x 4.4.1 #32416

Merged
merged 1 commit into from
Apr 19, 2023
Merged

Update to Vert.x 4.4.1 #32416

merged 1 commit into from
Apr 19, 2023

Conversation

cescoffier
Copy link
Member

  • Update Netty to version 4.1.90.Final
  • Update Vert.x to version 4.4.1
  • Update Mutiny bindings to version 3.3.0
  • Update Quarkus HTTP to version 5.0.2.Final

@quarkus-bot quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/vertx labels Apr 5, 2023
@quarkus-bot

This comment has been minimized.

@geoand
Copy link
Contributor

geoand commented Apr 5, 2023

CI seems to not like it:

2023-04-05T06:11:49.7367657Z [ERROR] Non-resolvable import POM: Could not find artifact io.smallrye.reactive:vertx-mutiny-clients-bom:pom:3.3.0-SNAPSHOT @ io.quarkus:quarkus-bom:999-SNAPSHOT, /home/runner/work/quarkus/quarkus/bom/application/pom.xml, line 414, column 25
2023-04-05T06:11:49.7368862Z [ERROR] Non-resolvable import POM: Could not find artifact io.smallrye.reactive:vertx-mutiny-clients-bom:pom:3.3.0-SNAPSHOT @ io.quarkus:quarkus-bom:999-SNAPSHOT, /home/runner/work/quarkus/quarkus/bom/application/pom.xml, line 414, column 25
2023-04-05T06:11:49.7369372Z 
2023-04-05T06:11:49.7369714Z     at org.apache.maven.project.DefaultProjectBuilder.build (DefaultProjectBuilder.java:397)
2023-04-05T06:11:49.7370407Z     at org.apache.maven.graph.DefaultGraphBuilder.collectProjects (DefaultGraphBuilder.java:414)
2023-04-05T06:11:49.7371198Z     at org.apache.maven.graph.DefaultGraphBuilder.getProjectsForMavenReactor (DefaultGraphBuilder.java:405)
2023-04-05T06:11:49.7371918Z     at org.apache.maven.graph.DefaultGraphBuilder.build (DefaultGraphBuilder.java:82)
2023-04-05T06:11:49.7380777Z     at org.apache.maven.DefaultMaven.buildGraph (DefaultMaven.java:535)
2023-04-05T06:11:49.7384110Z     at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:220)
2023-04-05T06:11:49.7387066Z     at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:193)
2023-04-05T06:11:49.7389947Z     at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:106)
2023-04-05T06:11:49.7393861Z     at org.apache.maven.cli.MavenCli.execute (MavenCli.java:963)
2023-04-05T06:11:49.7401227Z     at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:296)
2023-04-05T06:11:49.7404343Z     at org.apache.maven.cli.MavenCli.main (MavenCli.java:199)
2023-04-05T06:11:49.7407459Z     at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
2023-04-05T06:11:49.7410569Z     at jdk.internal.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:62)
2023-04-05T06:11:49.7417637Z     at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
2023-04-05T06:11:49.7423180Z     at java.lang.reflect.Method.invoke (Method.java:566)
2023-04-05T06:11:49.7426797Z     at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:282)
2023-04-05T06:11:49.7430851Z     at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:225)
2023-04-05T06:11:49.7434026Z     at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:406)
2023-04-05T06:11:49.7438679Z     at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:347)
2023-04-05T06:11:49.7441435Z     at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
2023-04-05T06:11:49.7444946Z     at jdk.internal.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:62)
2023-04-05T06:11:49.7458098Z     at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
2023-04-05T06:11:49.7460502Z     at java.lang.reflect.Method.invoke (Method.java:566)
2023-04-05T06:11:49.7462712Z     at org.apache.maven.wrapper.BootstrapMainStarter.start (BootstrapMainStarter.java:52)
2023-04-05T06:11:49.7464952Z     at org.apache.maven.wrapper.WrapperExecutor.execute (WrapperExecutor.java:161)
2023-04-05T06:11:49.7470901Z     at org.apache.maven.wrapper.MavenWrapperMain.main (MavenWrapperMain.java:73)
2023-04-05T06:11:49.7473251Z [ERROR]   
2023-04-05T06:11:49.7475187Z [ERROR]   The project io.quarkus:quarkus-bom:999-SNAPSHOT (/home/runner/work/quarkus/quarkus/bom/application/pom.xml) has 1 error
2023-04-05T06:11:49.7484158Z [ERROR]     Non-resolvable import POM: Could not find artifact io.smallrye.reactive:vertx-mutiny-clients-bom:pom:3.3.0-SNAPSHOT @ line 414, column 25 -> [Help 2]
2023-04-05T06:11:49.7487147Z org.apache.maven.model.resolution.UnresolvableModelException: Could not find artifact io.smallrye.reactive:vertx-mutiny-clients-bom:pom:3.3.0-SNAPSHOT

bom/application/pom.xml Outdated Show resolved Hide resolved
@gsmet
Copy link
Member

gsmet commented Apr 5, 2023

i turned it into draft. Mark it ready for review when completed.

@cescoffier cescoffier marked this pull request as ready for review April 6, 2023 09:45
@quarkus-bot

This comment has been minimized.

@Sanne
Copy link
Member

Sanne commented Apr 6, 2023

👍 seems no problems with this on Hibernate Reactive, but we might do a release of it just to match.

@quarkus-bot

This comment has been minimized.

@gsmet
Copy link
Member

gsmet commented Apr 7, 2023

Unfortunately, some of the CI failures look related.

@Sanne
Copy link
Member

Sanne commented Apr 7, 2023

Unfortunately, some of the CI failures look related.

Agreed. We'll hold of from upgrading HR until it's clarified

@cescoffier
Copy link
Member Author

The TCK one is annoying. @Sgitario can you help me with this one?
I've tried to update the AsyncInvokeImpl, but while I complete the completable future with a processing exception it still sees the other exception.

@Sgitario
Copy link
Contributor

The TCK one is annoying. @Sgitario can you help me with this one? I've tried to update the AsyncInvokeImpl, but while I complete the completable future with a processing exception it still sees the other exception.

Back from some PTO days. Will take a look asap.

@Sgitario
Copy link
Contributor

I have zero experience with these tests, but I'm seeing the following exception when running the SslMutualTest.shouldFailWithNoClientSignatureCDI:

2023-04-11 09:04:16,629 WARN  [io.qua.boo.BootstrapAppModelFactory] (main) Expected project directory /home/jcarvaja/sources/quarkus/quarkus/tcks/microprofile-rest-client-reactive/target/quarkus-arquillian-test15827915960220448922/app does not match current project directory /home/jcarvaja/sources/quarkus/quarkus/tcks/microprofile-rest-client-reactive
2023-04-11 09:04:20,679 WARN  [io.qua.con.build] (main) The 'quarkus.rest-client-reactive.disable-smart-produces' config property is deprecated and should not be used anymore
2023-04-11 09:04:20,681 WARN  [io.qua.con.build] (main) The 'quarkus.rest-client-reactive.scope' config property is deprecated and should not be used anymore
2023-04-11 09:04:23,438 INFO  [io.quarkus] (main) quarkus-tck-microprofile-rest-client-reactive 999-SNAPSHOT on JVM (powered by Quarkus 999-SNAPSHOT) started in 0.884s. Listening on: http://localhost:8081
2023-04-11 09:04:23,439 INFO  [io.quarkus] (main) Profile test activated. 
2023-04-11 09:04:23,440 INFO  [io.quarkus] (main) Installed features: [cdi, rest-client-reactive, resteasy-reactive, resteasy-reactive-jsonb, servlet, smallrye-context-propagation, vertx]
2023-04-11 09:04:24,697 ERROR [io.qua.arq.QuarkusDeployableContainer] (main) Failed to copy static field: java.lang.IllegalArgumentException: Can not set static org.eclipse.microprofile.rest.client.tck.ssl.HttpsServer field org.eclipse.microprofile.rest.client.tck.ssl.AbstractSslTest.httpsServer to org.eclipse.microprofile.rest.client.tck.ssl.HttpsServer
	at java.base/jdk.internal.reflect.UnsafeFieldAccessorImpl.throwSetIllegalArgumentException(UnsafeFieldAccessorImpl.java:167)
	at java.base/jdk.internal.reflect.UnsafeFieldAccessorImpl.throwSetIllegalArgumentException(UnsafeFieldAccessorImpl.java:171)
	at java.base/jdk.internal.reflect.UnsafeStaticObjectFieldAccessorImpl.set(UnsafeStaticObjectFieldAccessorImpl.java:79)
	at java.base/java.lang.reflect.Field.set(Field.java:799)
	at io.quarkus.arquillian.QuarkusDeployableContainer.deploy(QuarkusDeployableContainer.java:213)
	at org.jboss.arquillian.container.impl.client.container.ContainerDeployController$3.call(ContainerDeployController.java:151)
	at org.jboss.arquillian.container.impl.client.container.ContainerDeployController$3.call(ContainerDeployController.java:118)
	at org.jboss.arquillian.container.impl.client.container.ContainerDeployController.executeOperation(ContainerDeployController.java:239)
	at org.jboss.arquillian.container.impl.client.container.ContainerDeployController.deploy(ContainerDeployController.java:118)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

@Sgitario
Copy link
Contributor

I have zero experience with these tests, but I'm seeing the following exception when running the SslMutualTest.shouldFailWithNoClientSignatureCDI:

2023-04-11 09:04:16,629 WARN  [io.qua.boo.BootstrapAppModelFactory] (main) Expected project directory /home/jcarvaja/sources/quarkus/quarkus/tcks/microprofile-rest-client-reactive/target/quarkus-arquillian-test15827915960220448922/app does not match current project directory /home/jcarvaja/sources/quarkus/quarkus/tcks/microprofile-rest-client-reactive
2023-04-11 09:04:20,679 WARN  [io.qua.con.build] (main) The 'quarkus.rest-client-reactive.disable-smart-produces' config property is deprecated and should not be used anymore
2023-04-11 09:04:20,681 WARN  [io.qua.con.build] (main) The 'quarkus.rest-client-reactive.scope' config property is deprecated and should not be used anymore
2023-04-11 09:04:23,438 INFO  [io.quarkus] (main) quarkus-tck-microprofile-rest-client-reactive 999-SNAPSHOT on JVM (powered by Quarkus 999-SNAPSHOT) started in 0.884s. Listening on: http://localhost:8081
2023-04-11 09:04:23,439 INFO  [io.quarkus] (main) Profile test activated. 
2023-04-11 09:04:23,440 INFO  [io.quarkus] (main) Installed features: [cdi, rest-client-reactive, resteasy-reactive, resteasy-reactive-jsonb, servlet, smallrye-context-propagation, vertx]
2023-04-11 09:04:24,697 ERROR [io.qua.arq.QuarkusDeployableContainer] (main) Failed to copy static field: java.lang.IllegalArgumentException: Can not set static org.eclipse.microprofile.rest.client.tck.ssl.HttpsServer field org.eclipse.microprofile.rest.client.tck.ssl.AbstractSslTest.httpsServer to org.eclipse.microprofile.rest.client.tck.ssl.HttpsServer
	at java.base/jdk.internal.reflect.UnsafeFieldAccessorImpl.throwSetIllegalArgumentException(UnsafeFieldAccessorImpl.java:167)
	at java.base/jdk.internal.reflect.UnsafeFieldAccessorImpl.throwSetIllegalArgumentException(UnsafeFieldAccessorImpl.java:171)
	at java.base/jdk.internal.reflect.UnsafeStaticObjectFieldAccessorImpl.set(UnsafeStaticObjectFieldAccessorImpl.java:79)
	at java.base/java.lang.reflect.Field.set(Field.java:799)
	at io.quarkus.arquillian.QuarkusDeployableContainer.deploy(QuarkusDeployableContainer.java:213)
	at org.jboss.arquillian.container.impl.client.container.ContainerDeployController$3.call(ContainerDeployController.java:151)
	at org.jboss.arquillian.container.impl.client.container.ContainerDeployController$3.call(ContainerDeployController.java:118)
	at org.jboss.arquillian.container.impl.client.container.ContainerDeployController.executeOperation(ContainerDeployController.java:239)
	at org.jboss.arquillian.container.impl.client.container.ContainerDeployController.deploy(ContainerDeployController.java:118)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

Note that this error seems unrelated (though it should be fixed in the Arquillian extension side).
I'm trying to see why Vert.x was returning the expected IOException when jetty returned javax.net.ssl.SSLHandshakeException: Empty client certificate chain, and now it returns io.vertx.core.http.HttpClosedException: Connection was closed.

@gsmet
Copy link
Member

gsmet commented Apr 11, 2023

I looked at the Oracle Reactive Client failure in io.quarkus.reactive.oracle.client.ReactiveOracleReloadTest.testHotReplacement and the error is the following:

HTTP/1.1 500 Internal Server Error
Content-Type: text/plain;charset=UTF-8
content-length: 85

Expected class java.sql.SQLException, got class io.vertx.oracleclient.OracleException

Apparently, exceptions are not wrapped in the same way as they were before. Not sure if the test needs tweaking or if it's a regression.

/cc @tsegismont

@Sgitario
Copy link
Contributor

The root cause of this issue seems to be that Netty is not throwing the handshake failure exception. This was done in here.

To be precise, before these changes, the condition handshakePromise.tryFailure(cause) returned true and hence the exception was thrown, but after these changes, it returns false. This logic that returns either false or true is: https://github.com/netty/netty/blob/4.1/common/src/main/java/io/netty/util/concurrent/DefaultPromise.java#L632

This logic has not changed in ages, so it must be something else that causes the change of behaviour (not sure if this is caused by a change in Vert.x or Netty). So, we should ask for support first from somebody in their teams.

@Sanne
Copy link
Member

Sanne commented Apr 11, 2023

Thanks @gsmet
FWIW, Oracle reactive support is still marked "preview", so if we want this PR urgently we could disable such tests (but we'd still need a careful look at the other ones)

@Sanne
Copy link
Member

Sanne commented Apr 11, 2023

The root cause of this issue seems to be that Netty is not throwing the handshake failure exception. This was done in here.

To be precise, before these changes, the condition handshakePromise.tryFailure(cause) returned true and hence the exception was thrown, but after these changes, it returns false. This logic that returns either false or true is: https://github.com/netty/netty/blob/4.1/common/src/main/java/io/netty/util/concurrent/DefaultPromise.java#L632

This logic has not changed in ages, so it must be something else that causes the change of behaviour (not sure if this is caused by a change in Vert.x or Netty). So, we should ask for support first from somebody in their teams.

@franz1981 do you think you could help here?

@franz1981
Copy link
Contributor

franz1981 commented Apr 11, 2023

If the result is already set, there is not much that can be done (reading the code, it returns true when already set): we should track which changes already make it to be set or, If return false, what's already set to handle it right eg by inspection of the cause (that's a promise method to check if an exception is already set as result and which one)

@cescoffier
Copy link
Member Author

This PR updates to netty .90. Maybe we can skip this update and stay on 89 for now.

@Sgitario
Copy link
Contributor

I got a fix, not clean but seems to work.

It's a change in Vert.x but that change makes sense. It's then a rewrapping game in the rest client and the proxy that caused the issue.

Really nice catchup @cescoffier ! 5e8c6bf

@gsmet
Copy link
Member

gsmet commented Apr 18, 2023

There's a problem with the PR. It includes way too many commits.

@cescoffier
Copy link
Member Author

@gsmet oups! I will fix that after lunch.

- Update Netty to version 4.1.90.Final
- Update Vert.x to version 4.4.1
- Update Mutiny bindings to version 3.3.0
- Update Quarkus HTTP to version 5.0.2.Final
@cescoffier
Copy link
Member Author

Ok, single commit - all included.

@quarkus-bot
Copy link

quarkus-bot bot commented Apr 18, 2023

Failing Jobs - Building 81cc1ea

Status Name Step Failures Logs Raw logs
Native Tests - Security1 Build ⚠️ Check → Logs Raw logs

@cescoffier cescoffier requested a review from geoand April 19, 2023 06:55
Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@cescoffier cescoffier merged commit be9b0f3 into quarkusio:main Apr 19, 2023
@quarkus-bot quarkus-bot bot added this to the 3.1 - main milestone Apr 19, 2023
@Sanne
Copy link
Member

Sanne commented Apr 19, 2023

Nice! we'll upgrade Hibernate Reactive as well then.

@cescoffier cescoffier deleted the vertx-4.4.1 branch April 19, 2023 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/amazon-lambda area/arc Issue related to ARC (dependency injection) area/cli Related to quarkus cli (not maven/gradle/etc.) area/config area/core area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation area/gradle Gradle area/graphics area/graphql area/hibernate-orm Hibernate ORM area/hibernate-reactive Hibernate Reactive area/hibernate-validator Hibernate Validator area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure area/jbang Issues related to when using jbang.dev with Quarkus area/kubernetes area/mailer area/maven area/metrics area/mongodb area/oidc area/panache area/persistence OBSOLETE, DO NOT USE area/platform Issues related to definition and interaction with Quarkus Platform area/qute The template engine area/rest area/resteasy-classic area/scheduler area/smallrye area/testing area/tracing area/undertow area/vertx
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants