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 ERR only (P)SUBSCRIBE / (P)UNSUBSCRIBE / PING / QUIT allowed in t… #695

Merged
merged 4 commits into from
Jul 31, 2023

Conversation

Kilowhisky
Copy link
Contributor

Fixes issue #694

I've confirmed that StackExchange.Redis isn't squawking about the error anymore and the pub/sub open/close doesn't happen on the server anymore either.

@Kilowhisky
Copy link
Contributor Author

Hold on this PR actually, i found a way for it to start spitting out

System.InvalidOperationException: unexpected tracer reply to PING: BulkString: 26 bytes\r\n

Which tells me something isn't quite right with the implementation.

@Kilowhisky
Copy link
Contributor Author

Yea, guess i need to figure out how to return a bulkArray, please stand by....

 case RedisCommand.PING:
                        // there are two different PINGs; "interactive" is a +PONG or +{your message},
                        // but subscriber returns a bulk-array of [ "pong", {your message} ]
                        switch (result.Type)
                        {
                            case ResultType.SimpleString:
                                happy = result.IsEqual(CommonReplies.PONG);
                                break;
                            case ResultType.MultiBulk:
                                if (result.ItemsCount == 2)
                                {
                                    var items = result.GetItems();
                                    happy = items[0].IsEqual(CommonReplies.PONG) && items[1].Payload.IsEmpty;
                                }
                                else
                                {
                                    happy = false;
                                }
                                break;
                            default:
                                happy = false;
                                break;
                        }
                        break;

@Kilowhisky
Copy link
Contributor Author

Ready to merge

@tidwall tidwall merged commit f3c2cca into tidwall:master Jul 31, 2023
1 check passed
@Kilowhisky Kilowhisky deleted the fix/694 branch July 31, 2023 15:52
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.

2 participants