Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Bring back the fix for COMMAND responses introduced in #273 #323

Merged
merged 1 commit into from
Apr 17, 2023

Conversation

pje
Copy link
Contributor

@pje pje commented Apr 17, 2023

It seems the fix introduced by #273 was removed by #278.

This PR just brings back the fix introduced by #273—fixing the broken COMMAND response.

From #273:

When I use github.com/go-redis/redis/v8 redis.Command would return a error:

redis: can't parse array reply: "$9440"

The reason is that WriteBulk append $len(str) in front of the data. Correct response of Command should be $len(commands).

@pje
Copy link
Contributor Author

pje commented Apr 17, 2023

There's an integration test for COMMAND, but it's skipped:

func TestCommand(t *testing.T) {
t.Skip("not sure about this one yet")
testRaw(t, func(c *client) {
c.DoLoosely("COMMAND")
})
}

@alicebob
Copy link
Owner

Weird. Thanks for the fix!

@alicebob alicebob merged commit 2f1aeb3 into alicebob:master Apr 17, 2023
@alicebob
Copy link
Owner

I missed that one. The inttest is a bit hard to get right, iirc.

@pje pje deleted the pje/bring-back-273-fix branch April 17, 2023 20:39
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants