On the serf_error_callbacks branch: Add Serf error callback handling using a context callback with our session as the baton. Assumes that we use one Serf context per ra_serf session, which is currently true. [in subversion/libsvn_ra_serf] * ra_serf.h (svn_ra_serf__session_t::serf_error): New member. (svn_ra_serf__wrap_err_stack): New prototype and debug-mode macro. * serf.c (sess_error_callback): New; the Serf error callback. (svn_ra_serf__open, ra_serf_dup_session): Register the callback when the Serf context is created. * util.c: Optionally include serf_bucket_util.h; it declares the required prototype for serf_default_readline(). (conn_setup): When a new Serf SSL context was created, tell it to use our session's error callback. (svn_ra_serf__context_run): Report errors collected from Serf. (bucket_limited_readline): Put this function in a conditional block, otherwise we get a warning about an unused function when experimental Serf features are turned on. * util_error.c: Include stdarg.h. (format_error_message): New; hoist error mesage formatting from svn_ra_serf__wrap_err(). (svn_ra_serf__wrap_err): Delegate to format_error_message(). (svn_ra_serf__wrap_err_stack): Implement, using format_error_message(). git-svn-id: https://svn.apache.org/repos/asf/subversion/branches/serf_error_callbacks@1931269 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/subversion/libsvn_ra_serf/ra_serf.h b/subversion/libsvn_ra_serf/ra_serf.h index b850ea6..ba98d5d 100644 --- a/subversion/libsvn_ra_serf/ra_serf.h +++ b/subversion/libsvn_ra_serf/ra_serf.h
@@ -172,6 +172,10 @@ /* Error that we've received but not yet returned upstream. */ svn_error_t *pending_error; + /* Errors gathered from Serf error callbacks. + FIXME: Can we just use pending_error? */ + svn_error_t *serf_error; + /* List of authn types supported by the client.*/ int authn_types; @@ -1570,14 +1574,23 @@ apr_pool_t *result_pool, apr_pool_t *scratch_pool); -/** Wrap STATUS from an serf function. If STATUS is not serf error code, - * this is equivalent to svn_error_wrap_apr(). +/* Wrap STATUS from an serf function. If STATUS is not serf error code, + * this is equivalent to svn_error_wrap_apr(). */ svn_error_t * svn_ra_serf__wrap_err(apr_status_t status, const char *fmt, ...); +/* Like svn_ra_serf__wrap_err but also accept a child error to maintain + * an error stack. + */ +svn_error_t * +svn_ra_serf__wrap_err_stack(apr_status_t status, + svn_error_t *child, + const char *fmt, + ...); + /* Create a bucket that just returns DATA (with length LEN) and then returns the APR_EAGAIN status */ serf_bucket_t * @@ -1675,7 +1688,8 @@ /* Wrapper macros to collect file and line information */ #define svn_ra_serf__wrap_err \ (svn_error__locate(__FILE__,__LINE__), (svn_ra_serf__wrap_err)) - +#define svn_ra_serf__wrap_err_stack \ + (svn_error__locate(__FILE__,__LINE__), (svn_ra_serf__wrap_err_stack)) #endif #ifdef __cplusplus
diff --git a/subversion/libsvn_ra_serf/serf.c b/subversion/libsvn_ra_serf/serf.c index 6ac7886..70241f9 100644 --- a/subversion/libsvn_ra_serf/serf.c +++ b/subversion/libsvn_ra_serf/serf.c
@@ -481,6 +481,22 @@ major, minor, patch); } +#if defined(SVN__SERF_EXPERIMENTAL) && SERF_VERSION_AT_LEAST(1, 5, 0) +static apr_status_t +sess_error_callback(void *baton, + unsigned source, + apr_status_t status, + const char *message) +{ + const char* prefix = (source & SERF_ERROR_CB_SSL_CONTEXT) ? "TLS" : "NET"; + svn_ra_serf__session_t *const session = baton; + + session->serf_error = svn_ra_serf__wrap_err_stack( + status, session->serf_error, _("%s: %s"), prefix, message); + return SVN_NO_ERROR; +} +#endif + /* Implements svn_ra__vtable_t.open_session(). */ static svn_error_t * svn_ra_serf__open(svn_ra_session_t *session, @@ -520,7 +536,17 @@ serf_sess->cancel_baton = callback_baton; /* todo: reuse serf context across sessions */ + + /* odot: For now, it's better to keep context-per-session, otherwise + dealing with Serf's error callbacks will be a real pain. + It would be different if we could guarantee a single Serf + context per svn_client_ctx_t. */ serf_sess->context = serf_context_create(serf_sess->pool); +#if defined(SVN__SERF_EXPERIMENTAL) && SERF_VERSION_AT_LEAST(1, 5, 0) + serf_context_error_callback_set(serf_sess->context, + sess_error_callback, + serf_sess); +#endif SVN_ERR(svn_ra_serf__blncache_create(&serf_sess->blncache, serf_sess->pool)); @@ -775,6 +801,11 @@ /* conn_latency */ new_sess->context = serf_context_create(result_pool); +#if defined(SVN__SERF_EXPERIMENTAL) && SERF_VERSION_AT_LEAST(1, 5, 0) + serf_context_error_callback_set(new_sess->context, + sess_error_callback, + new_sess); +#endif SVN_ERR(load_config(new_sess, old_sess->config, result_pool, scratch_pool));
diff --git a/subversion/libsvn_ra_serf/util.c b/subversion/libsvn_ra_serf/util.c index 415a0a7..ebd977a 100644 --- a/subversion/libsvn_ra_serf/util.c +++ b/subversion/libsvn_ra_serf/util.c
@@ -31,6 +31,9 @@ #include <serf.h> #include <serf_bucket_types.h> +#if defined(SVN__SERF_EXPERIMENTAL) && SERF_VERSION_AT_LEAST(1, 5, 0) +#include <serf_bucket_util.h> +#endif #include "svn_hash.h" #include "svn_dirent_uri.h" @@ -549,6 +552,11 @@ ssl_server_cert_cb, conn); +#if defined(SVN__SERF_EXPERIMENTAL) && SERF_VERSION_AT_LEAST(1, 5, 0) + serf_ssl_use_context_error_callback(conn->ssl_context, + conn->session->context); +#endif + /* See if the user wants us to trust "default" openssl CAs. */ if (conn->session->trust_default_ca) { @@ -954,16 +962,20 @@ SVN_ERR(err); if (status) { + svn_error_t *const collected_errors = sess->serf_error; + sess->serf_error = NULL; + /* ### This omits SVN_WARNING, and possibly relies on the fact that ### MAX(SERF_ERROR_*) < SVN_ERR_BAD_CATEGORY_START? */ if (status >= SVN_ERR_BAD_CATEGORY_START && status < SVN_ERR_LAST) { /* apr can't translate subversion errors to text */ - SVN_ERR_W(svn_error_create(status, NULL, NULL), + SVN_ERR_W(svn_error_create(status, collected_errors, NULL), _("Error running context")); } - return svn_ra_serf__wrap_err(status, _("Error running context")); + return svn_ra_serf__wrap_err_stack(status, collected_errors, + _("Error running context")); } return SVN_NO_ERROR; @@ -2159,6 +2171,7 @@ return props; } +#if !defined(SVN__SERF_EXPERIMENTAL) || !SERF_VERSION_AT_LEAST(1, 5, 0) static apr_status_t bucket_limited_readline(serf_bucket_t *bucket, int acceptable, apr_size_t requested, int *found, @@ -2271,6 +2284,7 @@ return status; } +#endif apr_status_t svn_ra_serf__default_readline(serf_bucket_t *bucket, int acceptable,
diff --git a/subversion/libsvn_ra_serf/util_error.c b/subversion/libsvn_ra_serf/util_error.c index bce935a..284df89 100644 --- a/subversion/libsvn_ra_serf/util_error.c +++ b/subversion/libsvn_ra_serf/util_error.c
@@ -20,6 +20,9 @@ * under the License. * ==================================================================== */ + +#include <stdarg.h> + #include <serf.h> #include "svn_utf.h" @@ -41,18 +44,16 @@ #undef svn_error_quick_wrap #undef svn_error_wrap_apr #undef svn_ra_serf__wrap_err +#undef svn_ra_serf__wrap_err_stack -svn_error_t * -svn_ra_serf__wrap_err(apr_status_t status, - const char *fmt, - ...) + +static void +format_error_message(svn_error_t *err, + apr_status_t status, + const char *serf_err_msg, + const char *fmt, + va_list args) { - const char *serf_err_msg = serf_error_string(status); - svn_error_t *err; - va_list ap; - - err = svn_error_create(status, NULL, NULL); - if (serf_err_msg || fmt) { const char *msg; @@ -78,9 +79,7 @@ /* Append it to the formatted message. */ if (fmt) { - va_start(ap, fmt); - msg = apr_pvsprintf(err->pool, fmt, ap); - va_end(ap); + msg = apr_pvsprintf(err->pool, fmt, args); } else { @@ -96,6 +95,37 @@ err->message = msg; } } +} + +svn_error_t * +svn_ra_serf__wrap_err(apr_status_t status, + const char *fmt, + ...) +{ + svn_error_t *err; + va_list args; + + err = svn_error_create(status, NULL, NULL); + va_start(args, fmt); + format_error_message(err, status, serf_error_string(status), fmt, args); + va_end(args); + return err; +} + + +svn_error_t * +svn_ra_serf__wrap_err_stack(apr_status_t status, + svn_error_t *child, + const char *fmt, + ...) +{ + svn_error_t *err; + va_list args; + + err = svn_error_create(status, child, NULL); + va_start(args, fmt); + format_error_message(err, status, serf_error_string(status), fmt, args); + va_end(args); return err; }