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

Keycloak DevService: Improve error messages and documentation #36883

Merged
merged 1 commit into from
Nov 7, 2023

Conversation

Felk
Copy link
Contributor

@Felk Felk commented Nov 6, 2023

We used to experience a lot of flakiness due to the keycloak devservice starting, but doing so with errors and therefore causing all tests that were running with keycloak to emit 'IllegalArgument accessToken cannot be null'. The start-up errors were a bit cryptic and unspecific and looked like this:

2023-11-06 08:01:14,552 INFO  [tc.doc.xxx.0.1] (build-7) Container ourrepomirror.docker/keycloak/keycloak:22.0.1 started in PT3M46.054007184S
2023-11-06 08:01:18,712 ERROR [io.qua.oid.dep.dev.key.KeycloakDevServicesProcessor] (build-7) Admin token can not be acquired: null
2023-11-06 08:01:19,655 ERROR [io.qua.oid.dep.dev.key.KeycloakDevServicesProcessor] (build-7) Realm rdpro can not be created 401 - Unauthorized
2023-11-06 08:01:27,690 ERROR [io.qua.oid.dep.dev.key.KeycloakDevServicesProcessor] (build-7) Realm rdpro can not be created: Keycloak server is not available: Retries exhausted : 5 attempts against 1699257689687/1699257689656 expiration
2023-11-06 08:01:27,699 INFO  [io.qua.oid.dep.dev.key.KeycloakDevServicesProcessor] (build-7) Dev Services for Keycloak started.

It took us some time to realize that Admin token can not be acquired: null actually meant 'Acquiring the Admin token timed out' and that 'null' was the TimeoutException's message. Thankfully the property quarkus.oidc.devui.web-client-timeout exists to increase this timeout, though it is somewhat oddly named ("devui"? Should these properties be split into two?).

I tried to improve the situation by making errors actually print the exception and stacktrace, and by improving the user feedback a bit.

@quarkus-bot quarkus-bot bot added the area/oidc label Nov 6, 2023
@Felk Felk force-pushed the keycloak-devservice-client-timeouts branch from 5aa55a7 to 88fa7b1 Compare November 6, 2023 10:55
Copy link
Member

@sberyozkin sberyozkin left a comment

Choose a reason for hiding this comment

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

Thanks

This comment has been minimized.

@sberyozkin
Copy link
Member

@Felk Can you please follow up with a formatting fix and squash the commits afterwards ?

@Felk Felk force-pushed the keycloak-devservice-client-timeouts branch from 88fa7b1 to 77fcb1f Compare November 6, 2023 12:47
@Felk
Copy link
Contributor Author

Felk commented Nov 6, 2023

Sure! I amended the formatting fix

This comment has been minimized.

@sberyozkin sberyozkin force-pushed the keycloak-devservice-client-timeouts branch from 77fcb1f to 672348a Compare November 6, 2023 17:37
@sberyozkin
Copy link
Member

sberyozkin commented Nov 6, 2023

@Felk I did another formatting update and also removed the description of the problem which was a copy and paste of this PR's description from the commit message, thanks

Copy link

quarkus-bot bot commented Nov 6, 2023

✔️ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

@gsmet gsmet merged commit fddb8fc into quarkusio:main Nov 7, 2023
21 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.6 - main milestone Nov 7, 2023
@gsmet
Copy link
Member

gsmet commented Nov 7, 2023

Thanks for taking the time to handle that.

@Felk Felk deleted the keycloak-devservice-client-timeouts branch November 7, 2023 09:25
@gsmet gsmet modified the milestones: 3.6 - main, 3.5.1 Nov 7, 2023
@aloubyansky aloubyansky modified the milestones: 3.5.1, 3.2.9.Final Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants