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: return underlying sasl error message #2164

Merged
merged 2 commits into from
Feb 27, 2022
Merged

Conversation

dnwe
Copy link
Collaborator

@dnwe dnwe commented Feb 25, 2022

The SASL Authentication response message from Kafka has an additional field which can contain a description of why the authentication failed. Currently we drop this in Sarama and just return a generic message based on the kError code.

@dnwe
Copy link
Collaborator Author

dnwe commented Feb 25, 2022

@k-wall thoughts?

aside: I wonder if we need to tweak/condense the output as it comes from Sarama on the sentinel / multi error case:

kafka: client has run out of available brokers to talk to (Is your cluster reachable?): 3 errors occurred:
        * kafka server: SASL Authentication failed.: 1 error occurred:
        * Authentication failed, invalid credentials


        * kafka server: SASL Authentication failed.: 1 error occurred:
        * Authentication failed, invalid credentials


        * kafka server: SASL Authentication failed.: 1 error occurred:
        * Authentication failed, invalid credentials

@dnwe dnwe force-pushed the dnwe/return-sasl-failure-message branch from f7a62e9 to 4862c46 Compare February 27, 2022 10:58
@dnwe
Copy link
Collaborator Author

dnwe commented Feb 27, 2022

Added a commit to condense the default output a little which now gives:

kafka: client has run out of available brokers to talk to (Is your cluster reachable?): 3 errors occurred:
        * kafka server: SASL Authentication failed.: Authentication failed, invalid credentials
        * kafka server: SASL Authentication failed.: Authentication failed, invalid credentials
        * kafka server: SASL Authentication failed.: Authentication failed, invalid credentials

dnwe added 2 commits February 27, 2022 13:32
The SASL Authentication response message from Kafka has an additional
field which can contain a description of why the authentication failed.
Currently we drop this in Sarama and just return a generic message based
on the kError code.
Add a customised default implementation of the multierror format func
that is less verbose for single error multierrors (return them without
the unnecessary "1 error occured:" prefix) and also condenses the output
by removing one of the newline separators from between multiple errors
in the output.
@dnwe dnwe force-pushed the dnwe/return-sasl-failure-message branch from 4862c46 to f436782 Compare February 27, 2022 13:33
@dnwe dnwe merged commit 3f90bc7 into main Feb 27, 2022
@dnwe dnwe deleted the dnwe/return-sasl-failure-message branch February 27, 2022 14:06
@dnwe dnwe added the fix label Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant