On Mon, 07 Nov 2011 13:48:46 -0700, Alex Rousskov wrote:
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?
That in this case we seem to have both an errno and an extended error
with values. Its not clear to me whether this change is keeping the
system errno around for the report tokens which display pure errno (or
'-') rather than our extended err_type.
Not a major issue since I think our err_type is a clearer message
anyway.
Amos
