On 11/05/2011 04:15 PM, Amos Jeffries wrote:
> On 2/11/2011 11:13 p.m., Tsantilas Christos wrote:
>> Currently the %err_detail access_log formating code does not display
>> something useful for the system admin in the case of the certificate
>> validation errors.
>>
>> This patch in the case of an ERR_SECURE_CONNECT_FAIL error displays
>> the certificate validation error name.
>>
> 
> +1. Looks okay.
> 
> I'm  a little dubious about passing request->detailError() the SSL error
> code instead of the errno. But have no strong objections.

Error detailing code was specifically designed to record
context-specific details beyond errno (which was already available in
most cases) and the request->detailError() method itself is usually used
to store non-errno details:

> ./src/Server.cc:            request->detailError(ERR_ICAP_FAILURE, 
> ERR_DETAIL_RESPMOD_BLOCK_LATE);
> ./src/client_side_request.cc:    request->detailError(ERR_ACCESS_DENIED, 
> ERR_DETAIL_REQMOD_BLOCK);
> ./src/client_side_request.cc:        request->detailError(ERR_ICAP_FAILURE, 
> ERR_DETAIL_CLT_REQMOD_RESP_BODY);
> ./src/client_side_request.cc:    request->detailError(ERR_ICAP_FAILURE, 
> errDetail);


We even document HttpRequest::errDetail to be errType-specific:

>     err_type errType;
>     int errDetail; ///< errType-specific detail about the transaction error


Why are you dubious about passing request->detailError() the SSL error code?


Thank you,

Alex.

Reply via email to