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

Config root not initialized yet #22684

Closed
rvansa opened this issue Jan 6, 2022 · 11 comments · Fixed by #22768
Closed

Config root not initialized yet #22684

rvansa opened this issue Jan 6, 2022 · 11 comments · Fixed by #22768
Labels
area/rest-client kind/bug Something isn't working
Milestone

Comments

@rvansa
Copy link
Contributor

rvansa commented Jan 6, 2022

Describe the bug

After upgrading 2.3.0 -> 2.6.1 my app won't start (at least in dev mode):

Caused by: java.lang.RuntimeException: Error injecting io.hyperfoil.tools.horreum.grafana.GrafanaClient io.hyperfoil.tools.horreum.server.GrafanaUserFilter.grafana
...
Caused by: javax.enterprise.inject.CreationException: Config root [io.quarkus.restclient.config.RestClientsConfig] with config phase [RUN_TIME] not initialized yet.

The client is used defined as below and configured in application.properties:

@WebFilter(value = "/*", asyncSupported = true)
@ApplicationScoped
public class GrafanaUserFilter extends HttpFilter {
   @Inject @RestClient
   GrafanaClient grafana;
}

@RegisterRestClient(configKey = "horreum.grafana")
public interface GrafanaClient { /* ... */ }
horreum.grafana/mp-rest/url=http://localhost:4040
horreum.grafana/mp-rest/scope=javax.inject.Singleton

Expected behavior

Application starts as it used to.

Actual behavior

Application fails to start.

How to Reproduce?

Fetch my project at this commit: Hyperfoil/Horreum@c5184be
and change <quarkus.version>2.3.0.Final</quarkus.version> to <quarkus.version>2.6.1.Final</quarkus.version> in root pom.xml.

Output of uname -a or ver

Linux rvansa 5.15.6-200.fc35.x86_64 #1 SMP Wed Dec 1 13:41:10 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

Output of java -version

OpenJDK 64-Bit Server VM 18.9 (build 11.0.13+8, mixed mode, sharing)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.6.1

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

No response

Additional information

I've bisected the change and it appears to have happened 2.3.1 -> 2.4.0.

@rvansa rvansa added the kind/bug Something isn't working label Jan 6, 2022
@radcortez radcortez self-assigned this Jan 6, 2022
@radcortez
Copy link
Member

This seems to be cause by #17220. The issue here is that Filters are initialised during static init, and the rest client configuration is only available in runtime.

Not sure what we can do here. Even if it was working before, because the config instance is not the same between static / runtime, the rest client could be configured differently depending on the phase (for instance if a source would override something between static / runtime). I'm wondering if we should provide the rest client root for static as well (even if it may be different).

Any thoughts @TomasHofman?

@radcortez
Copy link
Member

As a workaround, the client could be created programatically, but that means that you need to provide the configuration yourself (and the register annotation has to be dropped).

@rvansa
Copy link
Contributor Author

rvansa commented Jan 7, 2022

@radcortez When you say that the filters are initialized during static init does that mean that any custom @ConfigProperty I'll use there will be fixed at compile time? Is the user warned about that? Where can I find the list of components that are initialized during this time? Could I use ContainerRequestFilter instead of servlet-based filter?

If you are able to supply a "fake" client during initialization and replace it with the real, runtime-configured one later on I think it's fine as there's no user code running that would use the injected fake. If you'd inject it with something that does not reflect runtime configuration that would be very confusing, straight failure is a better alternative.

In any case it would be nice to detect this problem and provide a more explanatory error message that would hint me what to do.

@radcortez
Copy link
Member

@radcortez When you say that the filters are initialized during static init does that mean that any custom @ConfigProperty I'll use there will be fixed at compile time?

No. When you run the application we actually create two Config instances. One for static and another for runtime. The difference between both is that the static one may have fewer sources and fewer configurations available. For instance, we cannot add a database-based ConfigSource because you fall into a chicken-egg problem. You need the config to configure the database, but you need the database to read the config.

Where can I find the list of components that are initialized during this time? Could I use ContainerRequestFilter instead of servlet-based filter?

Not sure if we have one, but on the REST side, all Providers (like filters) are initialized during static init.

If you are able to supply a "fake" client during initialization and replace it with the real, runtime-configured one later on I think it's fine as there's no user code running that would use the injected fake. If you'd inject it with something that does not reflect runtime configuration that would be very confusing, straight failure is a better alternative.

The problem with the new REST Configuration is that the Config Root is marked specifically to be only for runtime (not static). Creating the client programmatically and passing in the configuration manually should work. Maybe wrapping the client in a Provider works as well. Not sure when the instance provider is initialized in that case, but we could try it out.

In any case it would be nice to detect this problem and provide a more explanatory error message that would hint me what to do.

Lets have a look into it and see if we can detect such cases to provide a more clear error message.

@TomasHofman
Copy link
Contributor

We did some further changes to rest client configuration, but I'm not sure which version of quarkus that landed in. The clients' config map was originally loaded as part of the RestClientsConfig Config Root, but that was later changed and the map is created programmatically now, based on properties read directly from Config instance. We had to make that change because of other issues we had with Config Roots. That change should make the configuration behave closer to how it was behaving originally.

Also, the RestClientsConfig Config Root is almost empty now, so it doesn't bring a large added value. Perhaps we can remove it altogether, if it helps.

I will try to do some experiments.

@TomasHofman
Copy link
Contributor

The idea with encapsulating the client in Provider seems to work for me:

@WebFilter(value = "/*", asyncSupported = true)
@ApplicationScoped
public class TestFilter extends HttpFilter {

    Logger logger = Logger.getLogger(this.getClass());

    @Inject
    @RestClient
    Provider<CountriesService> countriesService;

    @Override
    protected void doFilter(HttpServletRequest req, HttpServletResponse res, FilterChain chain)
            throws IOException, ServletException {
        logger.infof("Filter run");
        logger.infof("Client response: %s", countriesService.get().getByName("vietnam"));
        super.doFilter(req, res, chain);
    }
}

@rvansa
Copy link
Contributor Author

rvansa commented Jan 7, 2022

@TomasHofman Confirmed, thanks a lot! This is a sensible workaround. Now the point of this issue should be improved error message that will suggest that approach.

@radcortez
Copy link
Member

I believe we could add an improved message here: io.quarkus.restclient.runtime.RestClientBase#getConfigRoot.

@TomasHofman
Copy link
Contributor

@radcortez that's a good idea. There is couple of more places I think, plus I will also check the reactive client. I will prepare a PR.

@rvansa thanks for confirming and for your input!

@varun-vajpayee
Copy link

JUST ADD @PreMatching
under @Provider annotation, this works for me
image

@paycraftdevops
Copy link

many thanks, worked for me too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/rest-client kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants