Commit ffa8666a authored by Pieter Noordhuis's avatar Pieter Noordhuis
Browse files

Change error reporting to have an explicit type

When there is an I/O error, errno should be used to find out what is
wrong. In other cases, errno cannot be used. So, use an explicit type in
Hiredis to define the different error scenarios that can occur.
parent 5db8008d
...@@ -47,8 +47,8 @@ static redisAsyncContext *redisAsyncInitialize(redisContext *c) { ...@@ -47,8 +47,8 @@ static redisAsyncContext *redisAsyncInitialize(redisContext *c) {
* an indirection to the redisContext struct. */ * an indirection to the redisContext struct. */
static void __redisAsyncCopyError(redisAsyncContext *ac) { static void __redisAsyncCopyError(redisAsyncContext *ac) {
redisContext *c = &(ac->c); redisContext *c = &(ac->c);
if (c->error != NULL) ac->err = c->err;
ac->error = c->error; ac->errstr = c->errstr;
} }
redisAsyncContext *redisAsyncConnect(const char *ip, int port) { redisAsyncContext *redisAsyncConnect(const char *ip, int port) {
...@@ -128,7 +128,7 @@ static void __redisAsyncDisconnect(redisAsyncContext *ac) { ...@@ -128,7 +128,7 @@ static void __redisAsyncDisconnect(redisAsyncContext *ac) {
/* Make sure error is accessible if there is any */ /* Make sure error is accessible if there is any */
__redisAsyncCopyError(ac); __redisAsyncCopyError(ac);
status = (ac->error == NULL) ? REDIS_OK : REDIS_ERR; status = (ac->err == 0) ? REDIS_OK : REDIS_ERR;
if (status == REDIS_OK) { if (status == REDIS_OK) {
/* When the connection is cleanly disconnected, there should not /* When the connection is cleanly disconnected, there should not
......
...@@ -54,8 +54,9 @@ typedef struct redisAsyncContext { ...@@ -54,8 +54,9 @@ typedef struct redisAsyncContext {
/* Hold the regular context, so it can be realloc'ed. */ /* Hold the regular context, so it can be realloc'ed. */
redisContext c; redisContext c;
/* Hold a reference to the error object so it can be used directly. */ /* Setup error flags so they can be used directly. */
char *error; int err;
char *errstr;
/* Called when the library expects to start reading/writing. /* Called when the library expects to start reading/writing.
* The supplied functions should be idempotent. */ * The supplied functions should be idempotent. */
......
...@@ -10,8 +10,8 @@ int main(void) { ...@@ -10,8 +10,8 @@ int main(void) {
redisReply *reply; redisReply *reply;
c = redisConnect((char*)"127.0.0.1", 6379); c = redisConnect((char*)"127.0.0.1", 6379);
if (c->error != NULL) { if (c->err) {
printf("Connection error: %s\n", c->error); printf("Connection error: %s\n", c->errstr);
exit(1); exit(1);
} }
......
...@@ -614,6 +614,17 @@ int redisFormatCommandArgv(char **target, int argc, const char **argv, const siz ...@@ -614,6 +614,17 @@ int redisFormatCommandArgv(char **target, int argc, const char **argv, const siz
return totlen; return totlen;
} }
static void __redisSetError(redisContext *c, int type, const char *str) {
c->err = type;
if (str) {
c->errstr = sdsnew(str);
} else {
/* Only REDIS_ERR_IO may lack a description! */
assert(type == REDIS_ERR_IO);
c->errstr = sdsnew(strerror(errno));
}
}
static int redisContextConnect(redisContext *c, const char *ip, int port) { static int redisContextConnect(redisContext *c, const char *ip, int port) {
char err[ANET_ERR_LEN]; char err[ANET_ERR_LEN];
if (c->flags & REDIS_BLOCK) { if (c->flags & REDIS_BLOCK) {
...@@ -623,11 +634,11 @@ static int redisContextConnect(redisContext *c, const char *ip, int port) { ...@@ -623,11 +634,11 @@ static int redisContextConnect(redisContext *c, const char *ip, int port) {
} }
if (c->fd == ANET_ERR) { if (c->fd == ANET_ERR) {
c->error = sdsnew(err); __redisSetError(c,REDIS_ERR_CONN,err);
return REDIS_ERR; return REDIS_ERR;
} }
if (anetTcpNoDelay(err,c->fd) == ANET_ERR) { if (anetTcpNoDelay(err,c->fd) == ANET_ERR) {
c->error = sdsnew(err); __redisSetError(c,REDIS_ERR_CONN,err);
return REDIS_ERR; return REDIS_ERR;
} }
return REDIS_OK; return REDIS_OK;
...@@ -635,7 +646,8 @@ static int redisContextConnect(redisContext *c, const char *ip, int port) { ...@@ -635,7 +646,8 @@ static int redisContextConnect(redisContext *c, const char *ip, int port) {
static redisContext *redisContextInit() { static redisContext *redisContextInit() {
redisContext *c = calloc(sizeof(redisContext),1); redisContext *c = calloc(sizeof(redisContext),1);
c->error = NULL; c->err = 0;
c->errstr = NULL;
c->obuf = sdsempty(); c->obuf = sdsempty();
c->fn = &defaultFunctions; c->fn = &defaultFunctions;
c->reader = NULL; c->reader = NULL;
...@@ -646,8 +658,8 @@ void redisFree(redisContext *c) { ...@@ -646,8 +658,8 @@ void redisFree(redisContext *c) {
/* Disconnect before free'ing if not yet disconnected. */ /* Disconnect before free'ing if not yet disconnected. */
if (c->flags & REDIS_CONNECTED) if (c->flags & REDIS_CONNECTED)
close(c->fd); close(c->fd);
if (c->error != NULL) if (c->errstr != NULL)
sdsfree(c->error); sdsfree(c->errstr);
if (c->obuf != NULL) if (c->obuf != NULL)
sdsfree(c->obuf); sdsfree(c->obuf);
if (c->reader != NULL) if (c->reader != NULL)
...@@ -702,14 +714,12 @@ int redisBufferRead(redisContext *c) { ...@@ -702,14 +714,12 @@ int redisBufferRead(redisContext *c) {
if (errno == EAGAIN) { if (errno == EAGAIN) {
/* Try again later */ /* Try again later */
} else { } else {
/* Set error in context */ __redisSetError(c,REDIS_ERR_IO,NULL);
c->error = sdscatprintf(sdsempty(),
"read: %s", strerror(errno));
return REDIS_ERR; return REDIS_ERR;
} }
} else if (nread == 0) { } else if (nread == 0) {
c->error = sdscatprintf(sdsempty(), __redisSetError(c,REDIS_ERR_EOF,
"read: Server closed the connection"); "Server closed the connection");
return REDIS_ERR; return REDIS_ERR;
} else { } else {
__redisCreateReplyReader(c); __redisCreateReplyReader(c);
...@@ -735,9 +745,7 @@ int redisBufferWrite(redisContext *c, int *done) { ...@@ -735,9 +745,7 @@ int redisBufferWrite(redisContext *c, int *done) {
if (errno == EAGAIN) { if (errno == EAGAIN) {
/* Try again later */ /* Try again later */
} else { } else {
/* Set error in context */ __redisSetError(c,REDIS_ERR_IO,NULL);
c->error = sdscatprintf(sdsempty(),
"write: %s", strerror(errno));
return REDIS_ERR; return REDIS_ERR;
} }
} else if (nwritten > 0) { } else if (nwritten > 0) {
...@@ -758,8 +766,7 @@ int redisBufferWrite(redisContext *c, int *done) { ...@@ -758,8 +766,7 @@ int redisBufferWrite(redisContext *c, int *done) {
static int __redisGetReply(redisContext *c, void **reply) { static int __redisGetReply(redisContext *c, void **reply) {
__redisCreateReplyReader(c); __redisCreateReplyReader(c);
if (redisReplyReaderGetReply(c->reader,reply) == REDIS_ERR) { if (redisReplyReaderGetReply(c->reader,reply) == REDIS_ERR) {
/* Copy the (protocol) error from the reader to the context. */ __redisSetError(c,REDIS_ERR_PROTOCOL,((redisReader*)c->reader)->error);
c->error = sdsnew(((redisReader*)c->reader)->error);
return REDIS_ERR; return REDIS_ERR;
} }
return REDIS_OK; return REDIS_OK;
......
...@@ -35,6 +35,15 @@ ...@@ -35,6 +35,15 @@
#define REDIS_ERR -1 #define REDIS_ERR -1
#define REDIS_OK 0 #define REDIS_OK 0
/* When an error occurs, the err flag in a context is set to hold the type of
* error that occured. REDIS_ERR_IO means there was an I/O error and you
* should use the "errno" variable to find out what is wrong.
* For other values, the "errstr" field will hold a description. */
#define REDIS_ERR_IO 1 /* error in read or write */
#define REDIS_ERR_CONN 2 /* error connecting */
#define REDIS_ERR_EOF 3 /* eof */
#define REDIS_ERR_PROTOCOL 4 /* protocol error */
/* Connection type can be blocking or non-blocking and is set in the /* Connection type can be blocking or non-blocking and is set in the
* least significant bit of the flags field in redisContext. */ * least significant bit of the flags field in redisContext. */
#define REDIS_BLOCK 0x1 #define REDIS_BLOCK 0x1
...@@ -87,8 +96,9 @@ struct redisContext; /* need forward declaration of redisContext */ ...@@ -87,8 +96,9 @@ struct redisContext; /* need forward declaration of redisContext */
typedef struct redisContext { typedef struct redisContext {
int fd; int fd;
int flags; int flags;
char *error; /* Error object is set when in erronous state */
char *obuf; /* Write buffer */ char *obuf; /* Write buffer */
int err; /* Error flags, 0 when there is no error */
char *errstr; /* String representation of error when applicable */
/* Function set for reply buildup and reply reader */ /* Function set for reply buildup and reply reader */
redisReplyObjectFunctions *fn; redisReplyObjectFunctions *fn;
......
...@@ -22,8 +22,8 @@ static long long usec(void) { ...@@ -22,8 +22,8 @@ static long long usec(void) {
static redisContext *blocking_context = NULL; static redisContext *blocking_context = NULL;
static void __connect(redisContext **target) { static void __connect(redisContext **target) {
*target = blocking_context = redisConnect((char*)"127.0.0.1", 6379); *target = blocking_context = redisConnect((char*)"127.0.0.1", 6379);
if (blocking_context->error != NULL) { if (blocking_context->err) {
printf("Connection error: %s\n", blocking_context->error); printf("Connection error: %s\n", blocking_context->errstr);
exit(1); exit(1);
} }
} }
...@@ -77,9 +77,10 @@ static void test_blocking_connection() { ...@@ -77,9 +77,10 @@ static void test_blocking_connection() {
__connect(&c); __connect(&c);
test("Returns I/O error when the connection is lost: "); test("Returns I/O error when the connection is lost: ");
reply = redisCommand(c,"QUIT"); reply = redisCommand(c,"QUIT");
test_cond(redisCommand(c,"PING") == NULL && test_cond(strcasecmp(reply->str,"OK") == 0 &&
strcasecmp(reply->str,"OK") == 0 && redisCommand(c,"PING") == NULL &&
strcmp(c->error,"read: Server closed the connection") == 0); c->err == REDIS_ERR_EOF &&
strcmp(c->errstr,"Server closed the connection") == 0);
freeReplyObject(reply); freeReplyObject(reply);
redisFree(c); redisFree(c);
......
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