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

Seems like FailureType is not correctly indicating authentication error #2362

Closed
tavisca-manpreet opened this issue Feb 6, 2023 · 14 comments · Fixed by #2367
Closed

Seems like FailureType is not correctly indicating authentication error #2362

tavisca-manpreet opened this issue Feb 6, 2023 · 14 comments · Fixed by #2367

Comments

@tavisca-manpreet
Copy link

No description provided.

@tavisca-manpreet tavisca-manpreet changed the title FailureType is not correctly indicating authentication error Seems like FailureType is not correctly indicating authentication error Feb 6, 2023
@tavisca-manpreet
Copy link
Author

tavisca-manpreet commented Feb 6, 2023

In case of incorrect credentials, I am getting StackExchange.Redis.RedisConnectionException exception. To differentiate Authentication errors, the code seems to have an enum of failure type. But in case of authentication error also, I am getting FailureType as "UnableToConnect".
image

@NickCraver
Copy link
Collaborator

NickCraver commented Feb 6, 2023

Which version of the library are you using?

For info: this was improved in 2.6.66+, release notes and links here: https://stackexchange.github.io/StackExchange.Redis/ReleaseNotes#2666

@tavisca-manpreet
Copy link
Author

I am using 2.6.90 version and the the screenshot is of the same code. ExceptionFactory seems to be setting same failure type in case of all UnableToConnect errors.

@NickCraver
Copy link
Collaborator

Can you post the actual error message and type you're seeing? Look at this from my view: all I have is a screenshot of code, I'm not sure what error you're seeing and what you're expecting to see. e.g. "is it including the test about it being an authentication failure?" - give us something to go on here...

@tavisca-manpreet
Copy link
Author

Exception message: It was not possible to connect to the redis server(s). There was an authentication failure; check that passwords (or client certificates) are configured correctly: (RedisServerException) Error: WRONGPASS invalid username-password pair or user is disabled.
Type: StackExchange.Redis.RedisConnectionException
image

@tavisca-manpreet
Copy link
Author

tavisca-manpreet commented Feb 6, 2023

I am trying to handle authentication failures in my code. To test this scenario, when I pass incorrect credentials , I am getting exception with the correct message but the "FailureType" in the exception seems to be not correct . I need a differentiator in case of authentication errors apart from the exception message. FailureType seems to be the correct field for it. But the value getting returned in this case in FailureType is "UnableToConnect" which I felt was incorrect as the enum has a better alternative to represent authentication failure.

@tavisca-vshah
Copy link

I agree with the @tavisca-manpreet point, enum StackExchange.Redis.ConnectionFailureType already have AuthenticationFailure Field. Expected behaviour should be to set FailureType to ConnectionFailureType.AuthenticationFailure when there is authetication failure.
image

@NickCraver
Copy link
Collaborator

Hey y'all - I'm just slammed with actual work stuff this week (this isn't the thing we're paid for :)) but wanted to say yes: completely agree. I haven't gotten a chance to look at this (hopefully later this week but we'll see how it goes), but I appreciate the clarification and additional detail. The request absolutely seems reasonable. Adding to the message it was because of auth and not a generic connection failure was something I only recently did a few months ago, and likely missed this aspect - will take a peek and report back.

NickCraver added a commit that referenced this issue Feb 8, 2023
I added to the string message a while back but we should set the ConnectionFailureType more specifically as well here so that people can handle it much easier. Fixing!
@NickCraver
Copy link
Collaborator

Had to wait on some builds this morning so poked at how big this is - super minor to adjust/test, fix is up here: #2367

@tavisca-manpreet
Copy link
Author

Thanks @NickCraver for the quick fix.

@NickCraver
Copy link
Collaborator

@tavisca-manpreet one note here: you could check the type on InnerException if you wanted until this is available on NuGet, in case that unblocks :)

@deychandan
Copy link

Hi @NickCraver,
I'm getting following error while try to run my .Net Core WebAPI. But with the same configuration it's working in another project.
image
Redis version: StackExchange.Redis v2.0.601(tried with latest version but no luck)
Redis Config:
"RedisSettings": {
"Endpoint": "xxxxxxxxx:6379",
"Password": "xxxxxxxxxx",
"Ssl": false,
"AbortOnConnectFail": true,
"KeepAlive": 30,
"SyncTimeout": 3000,
"ConnectTimeout": 4000,
"ConnectRetry": 3,
"sslprotocols": "tls12"
}

Thanks
Chandan

@NickCraver
Copy link
Collaborator

I'm not sure how your connection is being created, but sslprotocols being specified while also turning SSL off is curious - any idea what's going on there?

@deychandan
Copy link

Hi @NickCraver
Sorry my bad wrongly added "sslprotocols". Following the config which is working in one project but not working in other project
"RedisSettings": {
"Endpoint": "sis-redis.development.us.markerspro.net:6379",
"Password": "bHoK4c2kHUuNj0WfnxmkWeVVo3DMVbnJ",
"Ssl": false,
"AbortOnConnectFail": true,
"KeepAlive": 30,
"SyncTimeout": 3000,
"ConnectTimeout": 4000,
"ConnectRetry": 3
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants