Rob, (Nick)

I have been looking a little more at the issue of returning meaningful error messages back to apache - i didn't like my previous mail's hijacking of apache as a solution - though it should provide an idea of what is going on. So now the implementation looks like. Rob, sorry - this should be a better way of doing exactly what my previous mail suggested - (caveat - I have built these, but not run them - the production machine is live)! However, we now have kept httpd separate from apr and so we don't need to mess with the includes and so on.

In srclib/apr-util/dbd/apr_dbd_mysql.c - rewrite dbd_mysql_check_conn, around line 740
--------------
static apr_status_t dbd_mysql_check_conn(apr_pool_t *pool, apr_dbd_t *handle) {
unsigned int mysql_err_num = mysql_ping(handle->conn);
if (mysql_err_num == 0) {
        return APR_SUCCESS;
} else {
return APR_OS_START_USERERR + mysql_err_num; /* we could also remove the CR_MIN_ERROR offset... */
}
}

--------------
Also in srclib/apr-util/dbd/apr_dbd_mysql.c - rewrite dbd_mysql_error, around line 240
--------------
static const char *dbd_mysql_error(apr_dbd_t *sql, int n) {
  const char *mysql_error_msg_string =  mysql_error(sql->conn);
     if (mysql_error_msg_string != NULL) {
          return mysql_error_msg_string;
     } else {
          switch (n) {
          case 0: return mysql_error_msg_string;
case CR_COMMANDS_OUT_OF_SYNC: return "mysql error COMMANDS_OUT_OF_SYNC"; case CR_SERVER_GONE_ERROR: return "mysql error SERVER_GONE_ERROR";
           case CR_UNKNOWN_ERROR: return "mysql error UNKNOWN_ERROR ";
/* add a whole load of other mysql errors here see mysql file errmsg.h */
          default: return "unspecified mysql error";
        }
     }
}
--------------
So we need to change return value of the apr_dbd_open function in srclib/apr-util/dbd/apr_dbd.c line 170. - The API is open regarding errors (lucky us) so rather than blanking the error out with a GEN, we can return rv
Change:
    if ((rv != APR_SUCCESS) && (rv != APR_ENOTIMPL)) {
        apr_dbd_close(driver, *handle);
        return APR_EGENERAL;
    }

To:
--------------
    if ((rv != APR_SUCCESS) && (rv != APR_ENOTIMPL)) {
        apr_dbd_close(driver, *handle);
        return rv;
    }
--------------
And finally we can change mod_dbd.c (modules/database/mod_dbd.c) around line 288
--------------
if (rec->driver == NULL) { ap_log_perror(APLOG_MARK, APLOG_CRIT, rv, rec->pool,"DBD: mod_dbd received a NULL when calling apr_dbd_get_driver for %s",svr->name); } rv = apr_dbd_open(rec->driver, rec->pool, svr->params, &rec- >handle); if (rec->driver == NULL) { ap_log_perror(APLOG_MARK, APLOG_CRIT, rv, rec->pool,"DBD: mod_dbd received a NULL when calling apr_dbd_get_driver for %s",svr->name); } rv = apr_dbd_open(rec->driver, rec->pool, svr->params, &rec- >handle);
    switch (rv) {
     case APR_SUCCESS: {
ap_log_perror(APLOG_MARK, APLOG_DEBUG, rv, rec->pool, "DBD: % s driver is successfully opened and connected", svr->name);
     } break;
     case APR_ENOTIMPL: {
ap_log_perror(APLOG_MARK, APLOG_CRIT, rv, rec->pool, "DBD: Can't connect to %s Not Implemented.", svr->name);
        return APR_ENOTIMPL;
     } break;
     default: {
const char *errmsg = apr_dbd_error(rec->driver, rec->handle, rv - APR_OS_START_USERERR); ap_log_perror(APLOG_MARK, APLOG_CRIT, rv, rec->pool,"DBD: Can't connect to %s - ", svr->name, (errmsg ? errmsg : "no reason given"));
        return APR_EGENERAL;
     } break;
    }
--------------
Nick, in modules/database/mod_dbd.c, just before we call apr_dbd_open is there any reason why not to check that rec->driver is not NULL ? The previous error management code only evaluates return values, and it seems possible to me that apr_dbd_get_driver can actually return a NULL driver without returning an error code to match. I am unsure as to whether or not the (rec->driver == NULL) test line is redundant.

Another challenge is how to pass the error result from mysql_ping (called in dbd_mysql_check_conn) over to dbd_mysql_error. I am not impressed with merely discarding it!! As the error is internal to mysql, it doesn't feel correct to be returned by apr_dbd_open, but we do have APR_OS_START_USERERR so I used that - is there a better way? Now we are handling any messages correctly, through the apr_dbd_error function, and we know that rec->driver is not null.

Of course the code above does a good deal more than merely deal with connection issues - it demonstrates how to use mysql error reporting from mod_dbd in a consistent manner. The significant change is making use of the mysql error numbers and returning them in APR_OS_START_USERERR space.

Hoping these mails are helping someone!
        -b



---------------------------------------------------------------------
The official User-To-User support forum of the Apache HTTP Server Project.
See <URL:http://httpd.apache.org/userslist.html> for more info.
To unsubscribe, e-mail: [EMAIL PROTECTED]
  "   from the digest: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to