Can you please give me a little credit? The problem here is not that
120171 doesn't get stringified in maintainer mode.
The problem here is that it is a number which _does not have_ a symbolic
name in APR's or Subversion's API, so if an API user wants to code
against it they need to either hard-code the number or #include <serf.h>
in their builds --- and last I checked, we didn't require API users to
do either of those. Compare this to sqlite: we wrap all SQLite error
integers to SVN_ERR_SQLITE_ERROR, except one or two which we decided to
be important so we get them their own svn error numbers.
And it's not hiding any codes, they are still in the chain as
err->child->apr_err (or svn_error_find_cause() if you prefer that).
Bert Huijben wrote on Tue, Jul 09, 2013 at 14:20:27 +0200:
>
>
> > -----Original Message-----
> > From: danielsh_at_apache.org [mailto:danielsh_at_apache.org]
> > Sent: dinsdag 9 juli 2013 04:38
> > To: commits_at_subversion.apache.org
> > Subject: svn commit: r1501049 - in /subversion/trunk/subversion:
> > include/svn_error_codes.h libsvn_ra_serf/util_error.c
> >
> > Author: danielsh
> > Date: Tue Jul 9 02:37:50 2013
> > New Revision: 1501049
> >
> > URL: http://svn.apache.org/r1501049
> > Log:
> > ra_serf: Do not return a serf apr_status_t.
> >
> > For me, this manifested as the following error chain (with the patch already
> > applied; before the patch, 120171 was the only code in the chain):
> >
> > % $svn info https://svn-us.apache.org/repos/asf
> > subversion/svn/info-cmd.c:663,
> > subversion/libsvn_client/info.c:300,
> > subversion/libsvn_client/ra.c:516,
> > subversion/libsvn_client/ra.c:393,
> > subversion/libsvn_ra/ra_loader.c:482:
> > (apr_err=SVN_ERR_RA_SERF_WRAPPED_ERROR)
> > svn: E230003: Unable to connect to a repository at URL 'https://svn-
> > us.apache.org/repos/asf'
> > subversion/libsvn_ra_serf/serf.c:506,
> > subversion/libsvn_ra_serf/options.c:508,
> > subversion/libsvn_ra_serf/util.c:817,
> > subversion/libsvn_ra_serf/util.c:784:
> > (apr_err=SVN_ERR_RA_SERF_WRAPPED_ERROR)
> > svn: E230003: Error running context: An error occurred during SSL
> > communication
> > subversion/libsvn_ra_serf/util.c:784: (apr_err=120171)
> > svn: E120171: APR does not understand this error code
> >
> > In that, 120171 is SERF_ERROR_SSL_COMM_FAILED, which is not a useful
> > value for
> > svn_error_t->apr_err (API users can't do anything with it).
>
> So without the patch a user sees:
>
> $ svn info https://svn.apache.org:80/
> svn: E120171: Unable to connect to a repository at URL 'https://svn.apache.org:80'
> svn: E120171: Error running context: An error occurred during SSL communication
>
> And with the patch
> $ svn info https://svn.apache.org:80/
> svn: E230003: Unable to connect to a repository at URL 'https://svn.apache.org:80'
> svn: E230003: Error running context: An error occurred during SSL communication
> svn: E120171: APR does not understand this error code
>
> (Connecting as HTTPS to any HTTP host will trigger this error)
>
> As AnkhSVN developer I would say this is a regression that should be fixed. And as a normal 'svn' user I would reason the same way.
>
>
> I prefer to see the clear error codes going through, and not adding errors of the error handling.
>
> That your enum value generator patch doesn't work for you is your and maybe 'svn'-s problem, but certainly not worth hiding proper full status codes (such as from the OS, Apr, Serf) from clients.
>
> Bert
>
Received on 2013-07-09 15:28:09 CEST