Author: brane
Date: Mon Jan 12 17:00:45 2026
New Revision: 1931269
Log:
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().
Modified:
subversion/branches/serf_error_callbacks/subversion/libsvn_ra_serf/ra_serf.h
subversion/branches/serf_error_callbacks/subversion/libsvn_ra_serf/serf.c
subversion/branches/serf_error_callbacks/subversion/libsvn_ra_serf/util.c
subversion/branches/serf_error_callbacks/subversion/libsvn_ra_serf/util_error.c
Modified: subversion/branches/serf_error_callbacks/subversion/libsvn_ra_serf/ra_serf.h
==============================================================================
--- subversion/branches/serf_error_callbacks/subversion/libsvn_ra_serf/ra_serf.h Mon Jan 12 16:32:44 2026 (r1931268)
+++ subversion/branches/serf_error_callbacks/subversion/libsvn_ra_serf/ra_serf.h Mon Jan 12 17:00:45 2026 (r1931269)
@@ -172,6 +172,10 @@ struct svn_ra_serf__session_t {
/* 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 @@ svn_ra_serf__create_sb_bucket(svn_spillb
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 @@ svn_ra_serf__default_readline(serf_bucke
/* 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
Modified: subversion/branches/serf_error_callbacks/subversion/libsvn_ra_serf/serf.c
==============================================================================
--- subversion/branches/serf_error_callbacks/subversion/libsvn_ra_serf/serf.c Mon Jan 12 16:32:44 2026 (r1931268)
+++ subversion/branches/serf_error_callbacks/subversion/libsvn_ra_serf/serf.c Mon Jan 12 17:00:45 2026 (r1931269)
@@ -481,6 +481,22 @@ get_user_agent_string(apr_pool_t *pool)
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 @@ svn_ra_serf__open(svn_ra_session_t *sess
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 @@ ra_serf_dup_session(svn_ra_session_t *ne
/* 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));
Modified: subversion/branches/serf_error_callbacks/subversion/libsvn_ra_serf/util.c
==============================================================================
--- subversion/branches/serf_error_callbacks/subversion/libsvn_ra_serf/util.c Mon Jan 12 16:32:44 2026 (r1931268)
+++ subversion/branches/serf_error_callbacks/subversion/libsvn_ra_serf/util.c Mon Jan 12 17:00:45 2026 (r1931269)
@@ -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 @@ conn_setup(apr_socket_t *sock,
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_ra_serf__context_run(svn_ra_serf__se
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 @@ svn_ra_serf__get_dirent_props(apr_uint32
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 @@ bucket_limited_readline(serf_bucket_t *b
return status;
}
+#endif
apr_status_t
svn_ra_serf__default_readline(serf_bucket_t *bucket, int acceptable,
Modified: subversion/branches/serf_error_callbacks/subversion/libsvn_ra_serf/util_error.c
==============================================================================
--- subversion/branches/serf_error_callbacks/subversion/libsvn_ra_serf/util_error.c Mon Jan 12 16:32:44 2026 (r1931268)
+++ subversion/branches/serf_error_callbacks/subversion/libsvn_ra_serf/util_error.c Mon Jan 12 17:00:45 2026 (r1931269)
@@ -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,
- ...)
-{
- const char *serf_err_msg = serf_error_string(status);
- svn_error_t *err;
- va_list ap;
-
- err = svn_error_create(status, NULL, NULL);
+static void
+format_error_message(svn_error_t *err,
+ apr_status_t status,
+ const char *serf_err_msg,
+ const char *fmt,
+ va_list args)
+{
if (serf_err_msg || fmt)
{
const char *msg;
@@ -78,9 +79,7 @@ svn_ra_serf__wrap_err(apr_status_t statu
/* 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 @@ svn_ra_serf__wrap_err(apr_status_t statu
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;
}
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.