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.

Sign up for GitHub

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

Jump to bottom

Quarkus REST client is unable to determine base URI with MP-style properties after MP bump and I can't see anything documented #44067

Closed
michalvavrik opened this issue Oct 24, 2024 · 17 comments · Fixed by #44073
Closed

Quarkus REST client is unable to determine base URI with MP-style properties after MP bump and I can't see anything documented #44067

michalvavrik opened this issue Oct 24, 2024 · 17 comments · Fixed by #44073
Labels
area/rest-client kind/bug Something isn't working
Milestone
3.17.0.CR1

Comments

@michalvavrik
Copy link
Member

michalvavrik commented Oct 24, 2024 •
edited
Loading

Describe the bug

My app started failing after #43959. There is nothing in migration guide and I don't know if this is expected. Please provide context. Thanks

Expected behavior

Documentation or app works. Worked with 3.15.x.

Actual behavior

REST client is missing base URI after MP bump.

2024-10-24 10:16:37,494 ERROR [io.qua.ver.htt.run.QuarkusErrorHandler] (executor-thread-1) HTTP Request to /client-hello failed, error id: 110a707e-daa0-420f-9843-50bfc2bae118-1: java.lang.RuntimeException: Error injecting org.acme.GreetingsService org.acme.GreetingClientResource.greetingsService
	at org.acme.GreetingClientResource_Bean.doCreate(Unknown Source)
	at org.acme.GreetingClientResource_Bean.create(Unknown Source)
	at org.acme.GreetingClientResource_Bean.create(Unknown Source)
	at io.quarkus.arc.impl.AbstractSharedContext.createInstanceHandle(AbstractSharedContext.java:119)
	at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:38)
	at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:35)
	at io.quarkus.arc.impl.LazyValue.get(LazyValue.java:32)
	at io.quarkus.arc.impl.ComputingCache.computeIfAbsent(ComputingCache.java:69)
	at io.quarkus.arc.impl.ComputingCacheContextInstances.computeIfAbsent(ComputingCacheContextInstances.java:19)
	at io.quarkus.arc.impl.AbstractSharedContext.get(AbstractSharedContext.java:35)
	at org.acme.GreetingClientResource_Bean.get(Unknown Source)
	at org.acme.GreetingClientResource_Bean.get(Unknown Source)
	at io.quarkus.arc.impl.ArcContainerImpl.beanInstanceHandle(ArcContainerImpl.java:559)
	at io.quarkus.arc.impl.ArcContainerImpl.beanInstanceHandle(ArcContainerImpl.java:539)
	at io.quarkus.arc.impl.ArcContainerImpl.beanInstanceHandle(ArcContainerImpl.java:572)
	at io.quarkus.arc.impl.ArcContainerImpl$3.get(ArcContainerImpl.java:331)
	at io.quarkus.arc.impl.ArcContainerImpl$3.get(ArcContainerImpl.java:328)
	at io.quarkus.arc.runtime.BeanContainerImpl$1.create(BeanContainerImpl.java:58)
	at io.quarkus.resteasy.reactive.common.runtime.ArcBeanFactory.createInstance(ArcBeanFactory.java:27)
	at org.jboss.resteasy.reactive.server.handlers.InstanceHandler.handle(InstanceHandler.java:26)
	at io.quarkus.resteasy.reactive.server.runtime.QuarkusResteasyReactiveRequestContext.invokeHandler(QuarkusResteasyReactiveRequestContext.java:139)
	at org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.run(AbstractResteasyReactiveContext.java:147)
	at io.quarkus.vertx.core.runtime.VertxCoreRecorder$14.runWith(VertxCoreRecorder.java:627)
	at org.jboss.threads.EnhancedQueueExecutor$Task.doRunWith(EnhancedQueueExecutor.java:2675)
	at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2654)
	at org.jboss.threads.EnhancedQueueExecutor.runThreadBody(EnhancedQueueExecutor.java:1627)
	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1594)
	at org.jboss.threads.DelegatingRunnable.run(DelegatingRunnable.java:11)
	at org.jboss.threads.ThreadLocalResettingRunnable.run(ThreadLocalResettingRunnable.java:11)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.base/java.lang.Thread.run(Thread.java:1583)
Caused by: java.lang.IllegalArgumentException: Unable to determine the proper baseUrl/baseUri. Consider registering using @RegisterRestClient(baseUri="someuri"), @RegisterRestClient(configKey="orkey"), or by adding 'quarkus.rest-client."org.acme.GreetingsService".url' or 'quarkus.rest-client."org.acme.GreetingsService".uri' to your Quarkus configuration
	at io.quarkus.rest.client.reactive.runtime.RestClientCDIDelegateBuilder.configureBaseUrl(RestClientCDIDelegateBuilder.java:379)
	at io.quarkus.rest.client.reactive.runtime.RestClientCDIDelegateBuilder.configureBuilder(RestClientCDIDelegateBuilder.java:76)
	at io.quarkus.rest.client.reactive.runtime.RestClientCDIDelegateBuilder.build(RestClientCDIDelegateBuilder.java:71)
	at io.quarkus.rest.client.reactive.runtime.RestClientCDIDelegateBuilder.createDelegate(RestClientCDIDelegateBuilder.java:52)
	at io.quarkus.rest.client.reactive.runtime.RestClientReactiveCDIWrapperBase.delegate(RestClientReactiveCDIWrapperBase.java:76)
	at io.quarkus.rest.client.reactive.runtime.RestClientReactiveCDIWrapperBase.<init>(RestClientReactiveCDIWrapperBase.java:30)
	at org.acme.GreetingsService$$CDIWrapper.<init>(Unknown Source)
	at org.acme.GreetingsService$$CDIWrapper_ClientProxy.<init>(Unknown Source)
	at org.acme.GreetingsService$$CDIWrapper_Bean.proxy(Unknown Source)
	at org.acme.GreetingsService$$CDIWrapper_Bean.get(Unknown Source)
	at org.acme.GreetingsService$$CDIWrapper_Bean.get(Unknown Source)
	... 31 more

How to Reproduce?

Steps to reproduce the behavior:

quarkus create app --extensions=rest-client,quarkus-rest
cd code-with-quarkus/

cat <<EOF > src/main/java/org/acme/GreetingsService.java
package org.acme;

import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.MediaType;
import org.eclipse.microprofile.rest.client.inject.RegisterRestClient;

@RegisterRestClient
public interface GreetingsService {

    @Path("/hello")
    @GET
    @Produces(MediaType.TEXT_PLAIN)
    String hello();
    
}
EOF

cat <<EOF > src/main/java/org/acme/GreetingClientResource.java
package org.acme;

import jakarta.inject.Inject;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.MediaType;
import org.eclipse.microprofile.rest.client.inject.RestClient;

@Path("/client-hello")
public class GreetingClientResource {
    
    @RestClient
    @Inject
    GreetingsService greetingsService;

    @GET
    @Produces(MediaType.TEXT_PLAIN)
    public String hello() {
        return greetingsService.hello();
    }
}
EOF

cat <<EOF > src/test/java/org/acme/GreetingClientResourceTest.java
package org.acme;

import io.quarkus.test.junit.QuarkusTest;
import org.junit.jupiter.api.Test;

import static io.restassured.RestAssured.given;
import static org.hamcrest.CoreMatchers.is;

@QuarkusTest
class GreetingClientResourceTest {
    @Test
    void testHelloEndpoint() {
        given()
          .when().get("/client-hello")
          .then()
             .statusCode(200)
             .body(is("Hello from Quarkus REST"));
    }

}
EOF

cat <<EOF > src/main/resources/application.properties
GreetingsService/mp-rest/url=http://localhost:8081
GreetingsService/mp-rest/scope=jakarta.inject.Singleton
EOF


PASS: mvn clean test
FAIL: mvn clean test -Dquarkus.platform.version=999-SNAPSHOT -Dquarkus.platform.group-id=io.quarkus

Output of uname -a or ver

Fedora

Output of java -version

Temurin 21

Quarkus version or git rev

999-SNAPSHOT

Build tool (ie. output of mvnw --version or gradlew --version)

Maven 3.9.4

Additional information

No response
@michalvavrik michalvavrik added area/rest-client kind/bug Something isn't working labels Oct 24, 2024
@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Oct 24, 2024

/cc @cescoffier (rest-client), @geoand (rest-client)

@michalvavrik michalvavrik mentioned this issue Oct 24, 2024
Merged
@michalvavrik michalvavrik changed the title Quarkus REST client is not registered with MP-style properties after MP bump and I can't see anything documented Quarkus REST client is unable to determine base URI with MP-style properties after MP bump and I can't see anything documented Oct 24, 2024
@geoand
Copy link
Contributor

geoand commented Oct 24, 2024

cc @radcortez

@michalvavrik
Copy link
Member Author

I created this issue in hurry, but for the context, this is issue now for both RESTEasy Classic client and Quarkus REST client. I suppose specs somehow changed, but if so, let's document Quarkus behavior changed as well, at least highlight the bump. Thanks

@radcortez
Copy link
Member

I'll have a look

@michalvavrik
Copy link
Member Author

I'll have a look

Thanks

@radcortez
Copy link
Member

You are using simple class names for the REST Client with MP properties. This is not described in the specification and only worked by accident:
https://download.eclipse.org/microprofile/microprofile-rest-client-4.0/microprofile-rest-client-spec-4.0.html#mpconfig

The amount of lookups performed by the REST Client for configuration was getting out of hand, so we restricted the amount of lookups. In fact, simple class name is not part of the documentation, but we did keep it for Quarkus style configuration quarkus.rest-client.*.

For MP style, it was never part of the specification and there is no documentation about it, so I'm unsure how you got to that configuration.

@radcortez
Copy link
Member

Any of these would work:

  • quarkus.rest-client."com.radcortez.quarkus.config.GreetingsService".uri
  • quarkus.rest-client."GreetingsService".uri
  • quarkus.rest-client.GreetingsService.uri
  • org.acme.GreetingsService/mp-rest/uri
  • quarkus.rest-client.[CONFIG-KEY-FROM-ANNOTATION].uri
  • [CONFIG-KEY-FROM-ANNOTATION]/mp-rest/uri

I can readd the Simple Name for MP style, but we never had a test for it in Quarkus, so #43345 never failed.

@michalvavrik
Copy link
Member Author

This test was added long before I came to QE and our colleague @Sgitario has moved quarkus-qe/quarkus-test-suite#2 to other job so I don't where it came from. In general, we usually do what we find in the docs and but I cannot say with certainty where it came from. If you agree to at least add a small note to the migration guide, I think we can close this. Unless you are sure it was never in Quarkus docs.

@michalvavrik
Copy link
Member Author

Fun fact @radcortez , setting @RegisterRestClient(configKey = "GreetingsService") leads to:

Caused by: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.rest.client.reactive.deployment.devservices.DevServicesRestClientHttpProxyProcessor#determineRequiredProxies threw an exception: io.smallrye.config.ConfigValidationException: Configuration validation failed:
	java.lang.IllegalStateException: Too many recursive interceptor actions
	java.lang.IllegalStateException: Too many recursive interceptor actions
	java.lang.IllegalStateException: Too many recursive interceptor actions
	at io.smallrye.config.SmallRyeConfig.buildMappings(SmallRyeConfig.java:139)
	at io.smallrye.config.SmallRyeConfig.<init>(SmallRyeConfig.java:93)
	at io.smallrye.config.SmallRyeConfigBuilder.build(SmallRyeConfigBuilder.java:771)
	at io.quarkus.restclient.config.RestClientsBuildTimeConfig.get(RestClientsBuildTimeConfig.java:121)
	at io.quarkus.rest.client.reactive.deployment.devservices.DevServicesRestClientHttpProxyProcessor.determineRequiredProxies(DevServicesRestClientHttpProxyProcessor.java:63)
	at java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:733)
	at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:856)
	at io.quarkus.builder.BuildContext.run(BuildContext.java:256)
	at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)
	at org.jboss.threads.EnhancedQueueExecutor$Task.doRunWith(EnhancedQueueExecutor.java:2675)
	at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2654)
	at org.jboss.threads.EnhancedQueueExecutor.runThreadBody(EnhancedQueueExecutor.java:1627)
	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1594)
	at java.base/java.lang.Thread.run(Thread.java:1583)
	at org.jboss.threads.JBossThread.run(JBossThread.java:499)

	at io.quarkus.builder.Execution.run(Execution.java:124)
	at io.quarkus.builder.BuildExecutionBuilder.execute(BuildExecutionBuilder.java:79)
	at io.quarkus.deployment.QuarkusAugmentor.run(QuarkusAugmentor.java:161)
	at io.quarkus.runner.bootstrap.AugmentActionImpl.runAugment(AugmentActionImpl.java:350)
	... 6 more
Caused by: io.smallrye.config.ConfigValidationException: Configuration validation failed:
	java.lang.IllegalStateException: Too many recursive interceptor actions
	java.lang.IllegalStateException: Too many recursive interceptor actions
	java.lang.IllegalStateException: Too many recursive interceptor actions
	at io.smallrye.config.SmallRyeConfig.buildMappings(SmallRyeConfig.java:139)
	at io.smallrye.config.SmallRyeConfig.<init>(SmallRyeConfig.java:93)
	at io.smallrye.config.SmallRyeConfigBuilder.build(SmallRyeConfigBuilder.java:771)
	at io.quarkus.restclient.config.RestClientsBuildTimeConfig.get(RestClientsBuildTimeConfig.java:121)
	at io.quarkus.rest.client.reactive.deployment.devservices.DevServicesRestClientHttpProxyProcessor.determineRequiredProxies(DevServicesRestClientHttpProxyProcessor.java:63)
	at java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:733)
	at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:856)
	at io.quarkus.builder.BuildContext.run(BuildContext.java:256)
	at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)
	at org.jboss.threads.EnhancedQueueExecutor$Task.doRunWith(EnhancedQueueExecutor.java:2675)
	at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2654)
	at org.jboss.threads.EnhancedQueueExecutor.runThreadBody(EnhancedQueueExecutor.java:1627)
	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1594)
	at java.base/java.lang.Thread.run(Thread.java:1583)
	at org.jboss.threads.JBossThread.run(JBossThread.java:499)

But linked Eclipse docs says I can use it. Did I misread it?

@radcortez
Copy link
Member

Initially, we only used the MP style configuration. Later, when we wanted to expand the configuration, we introduced the quarkus.rest-client namespace. Here is the commit:

78981c1

I found no references to Simple Name, so I believe it was never there. Even for quarkus.rest-client, there are no references to it, but it is indeed covered in our tests. I guess it is an undocumented feature.

When I was doing the change, I tried to be careful to follow both docs, our tests, and the specification, and since I couldn't find any references to a Simple Name MP style, I did remove it (hey, one less lookup :)), but I guess I could have kept it.

I'm fine to include a note about it.

@michalvavrik
Copy link
Member Author

I'm fine to include a note about it.

Thanks, I think it's better to follow specs. But I'll let @geoand to have a say. Personally I'll migrate our test.

@geoand
Copy link
Contributor

geoand commented Oct 24, 2024

Probably safer to add the note just in case

@radcortez
Copy link
Member

Fun fact @radcortez , setting @RegisterRestClient(configKey = "GreetingsService") leads to:

Hum, I guess this creates a clash with the quarkus.rest-client."GreetingsService".uri, because quarkus.rest-client.[CONFIG-KEY-FROM-ANNOTATION].uri will be rewritten to the exact same name. If you use any other name, it will work. I'll have to add a check in there.

@michalvavrik
Copy link
Member Author

Fun fact @radcortez , setting @RegisterRestClient(configKey = "GreetingsService") leads to:

Hum, I guess this creates a clash with the quarkus.rest-client."GreetingsService".uri, because quarkus.rest-client.[CONFIG-KEY-FROM-ANNOTATION].uri will be rewritten to the exact same name. If you use any other name, it will work. I'll have to add a check in there.

I only changed that one line in the reproducer, there is no quarkus.rest-client.... but I think I understand you, you somehow relocate it in the interceptor so this happens. In that case it is a bug and you will fix it, right? Sorry for being slow.

@radcortez
Copy link
Member

I only changed that one line in the reproducer, there is no quarkus.rest-client.... but I think I understand you, you somehow relocate it in the interceptor so this happens. In that case it is a bug and you will fix it, right? Sorry for being slow.

Correct. No need to be sorry, it was a good find :) I'll fix it.

@radcortez radcortez mentioned this issue Oct 24, 2024
Merged
@radcortez
Copy link
Member

Added this to the Migration Guide:
https://github.com/quarkusio/quarkus/wiki/Migration-Guide-3.17#rest-client

@michalvavrik
Copy link
Member Author

Thank you

@michalvavrik michalvavrik mentioned this issue Oct 24, 2024
Merged
9 tasks
@quarkus-bot quarkus-bot bot added this to the 3.17 - main milestone Oct 24, 2024
@radcortez radcortez mentioned this issue Dec 10, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/rest-client kind/bug Something isn't working
Projects
None yet
Milestone
3.17.0.CR1
3 participants
@geoand @radcortez @michalvavrik