On Wed, Jul 10, 2013 at 1:08 PM, Daniel Shahaf <d.s_at_daniel.shahaf.name> wrote:
>...
> Okay. Suppose an API caller gets 120171 as the error code from
> svn_ra_foo(). I ignore the question of how to recognise that 120171 is
> not a Subversion error[1].
>
> 1 - How does he know that 120171 is a serf error?
>
> 2 - Given that 120171 is a serf error, how does he know that it is "SSL
> communications related errors" (docstring of SERF_ERROR_SSL_COMM_FAILED)?
>
> 3 - How does such an API caller get an error string for that error code?
>
> What are your answers to these questions?
You asked for background. Done. Now, I'm going to artfully dodge
answering/responding those questions. :-D
I think my real answer is that the system wasn't very well-defined,
and nobody bothered to really push against it in the intervening
years. You're now attempting to correct for those problems. Fair
enough.
It might be easier to get consensus by addressing it in that light.
>...
> For 1 again, I suggest alternatively: add
> #define SVN_ERROR_IS_SERF_ERROR(status) \
> ((status) > APR_OS_START_USERERR+100 && \
> (status) < APR_OS_START_USERERR+200)
> to our public API. (I'm working off your statement that the reservation
> is one hundred codes.)
>= ... but yeah. I like this one. Our API is quite clear that serf is in there (ref: SVN_ERR_RA_SERF_CATEGORY_START). Or Berkeley DB. Or SQLite. I think it is fair to surface those aspects from svn.
> I don't have a specific idea for 2. Requiring API consumers to include
> serf.h in their builds is less than ideal:
I think an API client can say "I don't know this error. I'll treat it
like APR_EGENERAL and just throw the message at the user." With
SVN_ERR_IS_SERF_ERROR(), they can classify the error some more.
>...
> [2] As an aside:
> subversion/include/svn_error_codes.h:74: SVN_WARNING = APR_OS_START_USERERR + 1,
> modules/proxy/ajp.h:86:#define AJP_EOVERFLOW (APR_OS_START_USERERR + 1)
> include/apreq_error.h:53:#define APREQ_ERROR_GENERAL APR_OS_START_USERERR
> include/apreq_error.h:55:#define APREQ_ERROR_TAINTED (APREQ_ERROR_GENERAL + 1)
I might suggest a patch to apr_errno.h to state these users and their
ranges. And maybe it would be APR_ERR_IS_SERF_ERROR() and
APR_ERR_IS_SVN_ERROR().
And fix the error_codes doc to release its claim on the
[USERERR,USERERR+4999] range. And possibly reduce its claim of "100
categories". We use 23 right now. And none of those use 100 or more
codes, so we could change our definition to 100-code ranges (but we
need to consider whether people use range/category tests; that might
throw off compat). Maybe we say *new* categories are 100 codes, and
only claim (say) 27 more, for a total of 50 categories ever.
Cheers,
-g
Received on 2013-07-10 21:35:22 CEST