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

Don't ignore exceptions from tasks #849

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

ahus1
Copy link
Contributor

@ahus1 ahus1 commented Jun 13, 2024

According the the JavaDoc a future is also done when it failed. The current code will only show exceptions if a task that the code is waiting for is failing with an exception. Any other task that failed in the meantime is silently ignored.

Closes #847

@ahus1 ahus1 self-assigned this Jun 13, 2024
@ahus1 ahus1 marked this pull request as draft June 13, 2024 18:55
@ahus1 ahus1 force-pushed the is-847-dont-ignore-exceptions-from-tasks branch from 89486ce to 5dfbde5 Compare June 13, 2024 19:00
Closes keycloak#847

Signed-off-by: Alexander Schwartz <[email protected]>
@ahus1 ahus1 force-pushed the is-847-dont-ignore-exceptions-from-tasks branch from 5dfbde5 to 92f5089 Compare June 13, 2024 19:06
@ahus1 ahus1 marked this pull request as ready for review June 13, 2024 19:09
@ahus1
Copy link
Contributor Author

ahus1 commented Jun 13, 2024

@kami619 - you've been involved in the original issue, maybe you could review this PR?

@kami619 kami619 self-requested a review June 13, 2024 19:31
}
}
if (!failures.isEmpty()) {
RuntimeException ex = new RuntimeException("Some futures failed");
Copy link
Contributor

Choose a reason for hiding this comment

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

would it be possible to log more information about the type of failure which could help with the debug or would it log anyways ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code creates a Runtime Exception which then has a list of one or more suppressed exceptions. All of the suppressed exceptions will be logged to together with the message "Some futures failed".

The the following line for where the suppressed exceptions are added.

@ahus1 ahus1 requested a review from kami619 June 13, 2024 19:48
Copy link
Contributor

@mhajas mhajas left a comment

Choose a reason for hiding this comment

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

Thanks for the change. Let's see what is happening there tomorrow.

@ahus1 ahus1 merged commit 6255163 into keycloak:main Jun 13, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nigthly run contains errors for Post correct credentials step
3 participants