Re: svn commit: r1501049 - in /subversion/trunk/subversion: include/svn_error_codes.h libsvn_ra_serf/util_error.c

On Tue, Jul 9, 2013 at 4:20 PM, Bert Huijben <bert_at_qqmail.nl> wrote:
>> -----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.
>
I completely agree with Bert: fix seems to be wrong. The idea was to
always add explicit error message string to svn_error_t for Serf
codes. Probably we're missing call to svn_ra_serf__wrap_err()
somewhere.

-- 
Ivan Zhakov