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

fix: keycloak operator retry #122

Merged
merged 12 commits into from
Oct 29, 2024
Merged

fix: keycloak operator retry #122

merged 12 commits into from
Oct 29, 2024

Conversation

merll
Copy link
Contributor

@merll merll commented Oct 29, 2024

This PR changes the error handling in the Keycloak operator with minimal changes to the functionality, for having the operator retry on failures that were sometimes only logged to the console. This includes the following changes:

  • Errors are in most cases no longer handled gracefully within a function, but instead thrown into the outer call of retryOperation, which reattempts recalling them indefinitely (that part has not been altered).
  • retryOperation has been modified to re-attempt in a loop instead of calling itself recursively.
  • Error handling has been moved to a separate function extractError which wraps errors in WrappedError to avoid duplicate messages while escalating to retryOperation. In addition, it extracts only the relevant information from HttpErrors to avoid leaking excess data (e.g. auth headers) to the console.
  • Generally some function calls have been flattened.

@merll merll requested review from ferruhcihan and ElderMatt and removed request for ferruhcihan October 29, 2024 09:45
Copy link
Contributor

@ferruhcihan ferruhcihan left a comment

Choose a reason for hiding this comment

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

Tested with a cluster. It works as expected. 👍

Copy link
Collaborator

@CasLubbers CasLubbers left a comment

Choose a reason for hiding this comment

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

One small remark

src/tasks/keycloak/errors.ts Outdated Show resolved Hide resolved
@merll merll merged commit 4f5a0f4 into main Oct 29, 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.

4 participants