brainy commented on code in PR #9:
URL: https://github.com/apache/serf/pull/9#discussion_r2179708899
##########
buckets/ssl_buckets.c:
##########
@@ -334,10 +338,20 @@ detect_renegotiate(const SSL *s, int where, int ret)
static void log_ssl_error(serf_ssl_context_t *ctx)
{
- unsigned long e = ERR_get_error();
- serf__log(LOGLVL_ERROR, LOGCOMP_SSL, __FILE__, ctx->config,
- "SSL Error: %s\n", ERR_error_string(e, NULL));
+ unsigned long err;
+
+ while ((err = ERR_get_error())) {
+
+ serf__log(LOGLVL_ERROR, LOGCOMP_SSL, __FILE__, ctx->config,
+ "SSL Error: %s\n", ERR_error_string(err, NULL));
+ if (err && ctx->error_callback) {
+ char ebuf[256];
+ ERR_error_string_n(err, ebuf, sizeof(ebuf));
Review Comment:
I disagree with the "libraries should not be logging" sentiment. They
shouldn't dump to stdout or stderr, but Serf doesn't do that -- it writes to
whatever logging sink the application cares to provide, so basically integrates
with the application's logging infrastructure. I've found this to be extremely
useful for debugging; not everything can be done with error handling, no matter
how sophisticated. Because logging by its nature leaves an audit trail for
states that are not error conditions and can't even be captured when an error
occurs.
In this case, I agree, we shouldn't be doing both.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
