Unverified Commit 79ae5ffc authored by Michael Grunder's avatar Michael Grunder Committed by GitHub
Browse files

Fix protocol error (#1106)



Fix ProtocolError

This commit attempts to fix hiredis such that a recoverable write error
will be retried rather than throwing a hard error.

Since our read/write functions are now behind function pointers, we
specify semantically that a return value of < 0 is a hard error, 0 a
recoverable error, and > 0 a success.

Our default `redisNetRead` function was already doing something similar
so this also improves code consistency.

Resolves #961
Co-authored-by: default avatarMaksim Tuleika <maksim.tuleika@appcast.io>
parent 61b5b299
...@@ -986,8 +986,8 @@ int redisBufferRead(redisContext *c) { ...@@ -986,8 +986,8 @@ int redisBufferRead(redisContext *c) {
* successfully written to the socket. When the buffer is empty after the * successfully written to the socket. When the buffer is empty after the
* write operation, "done" is set to 1 (if given). * write operation, "done" is set to 1 (if given).
* *
* Returns REDIS_ERR if an error occurred trying to write and sets * Returns REDIS_ERR if an unrecoverable error occurred in the underlying
* c->errstr to hold the appropriate error string. * c->funcs->write function.
*/ */
int redisBufferWrite(redisContext *c, int *done) { int redisBufferWrite(redisContext *c, int *done) {
......
...@@ -249,10 +249,16 @@ typedef struct redisContextFuncs { ...@@ -249,10 +249,16 @@ typedef struct redisContextFuncs {
void (*free_privctx)(void *); void (*free_privctx)(void *);
void (*async_read)(struct redisAsyncContext *); void (*async_read)(struct redisAsyncContext *);
void (*async_write)(struct redisAsyncContext *); void (*async_write)(struct redisAsyncContext *);
/* Read/Write data to the underlying communication stream, returning the
* number of bytes read/written. In the event of an unrecoverable error
* these functions shall return a value < 0. In the event of a
* recoverable error, they should return 0. */
ssize_t (*read)(struct redisContext *, char *, size_t); ssize_t (*read)(struct redisContext *, char *, size_t);
ssize_t (*write)(struct redisContext *); ssize_t (*write)(struct redisContext *);
} redisContextFuncs; } redisContextFuncs;
/* Context for a connection to Redis */ /* Context for a connection to Redis */
typedef struct redisContext { typedef struct redisContext {
const redisContextFuncs *funcs; /* Function table */ const redisContextFuncs *funcs; /* Function table */
......
...@@ -70,7 +70,7 @@ ssize_t redisNetRead(redisContext *c, char *buf, size_t bufcap) { ...@@ -70,7 +70,7 @@ ssize_t redisNetRead(redisContext *c, char *buf, size_t bufcap) {
__redisSetError(c, REDIS_ERR_TIMEOUT, "recv timeout"); __redisSetError(c, REDIS_ERR_TIMEOUT, "recv timeout");
return -1; return -1;
} else { } else {
__redisSetError(c, REDIS_ERR_IO, NULL); __redisSetError(c, REDIS_ERR_IO, strerror(errno));
return -1; return -1;
} }
} else if (nread == 0) { } else if (nread == 0) {
...@@ -82,15 +82,19 @@ ssize_t redisNetRead(redisContext *c, char *buf, size_t bufcap) { ...@@ -82,15 +82,19 @@ ssize_t redisNetRead(redisContext *c, char *buf, size_t bufcap) {
} }
ssize_t redisNetWrite(redisContext *c) { ssize_t redisNetWrite(redisContext *c) {
ssize_t nwritten = send(c->fd, c->obuf, sdslen(c->obuf), 0); ssize_t nwritten;
nwritten = send(c->fd, c->obuf, sdslen(c->obuf), 0);
if (nwritten < 0) { if (nwritten < 0) {
if ((errno == EWOULDBLOCK && !(c->flags & REDIS_BLOCK)) || (errno == EINTR)) { if ((errno == EWOULDBLOCK && !(c->flags & REDIS_BLOCK)) || (errno == EINTR)) {
/* Try again later */ /* Try again */
return 0;
} else { } else {
__redisSetError(c, REDIS_ERR_IO, NULL); __redisSetError(c, REDIS_ERR_IO, strerror(errno));
return -1; return -1;
} }
} }
return nwritten; return nwritten;
} }
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment