Better reporting of REQMOD failures during body processing.

When REQMOD body processing fails, the server-side needs to abort the
in-progress transaction. Use HTTP 500 (Internal Server Error) instead of 502
(Bad Gateway) status code and a new custom error detail for this case.

There is no perfect status code for ICAP errors because some view ICAP
processing as an integral part of the proxy (closer to the internal
proxy code) and some treat it as an external entity (closer to an
"upstream" web server).

Nevertheless, 502 (Bad Gateway) feels like a worse fit because from web
user and traffic flow point of views ICAP is less of a "gateway" or
"upstream" server than it is an "internally" used auxiliary service.
When both status codes were used, we have received reasonable complaints
about 502 responses but not (IIRC) about 500 responses.

Using custom error detail is better than errno in this context because
errno is often not set for ICAP errors and because by the time we
generate an error, the errno value is likely to be from a different
system call error anyway.

diff --git a/src/err_detail_type.h b/src/err_detail_type.h
index 826617d25c0fdfd10d90536b3b12e664c6f82005..914925be3e74c6dab96e1a29d815cf0cb9f987f2 100644 (file)
--- a/src/err_detail_type.h
+++ b/src/err_detail_type.h
@@ -7,6 +7,7 @@ typedef enum {
     ERR_DETAIL_CLT_REQMOD_ABORT = ERR_DETAIL_START, // client-side detected transaction abort
     ERR_DETAIL_CLT_REQMOD_REQ_BODY, // client-side detected REQMOD request body adaptation failure
     ERR_DETAIL_CLT_REQMOD_RESP_BODY, // client-side detected REQMOD satisfaction reply body failure
+    ERR_DETAIL_SRV_REQMOD_REQ_BODY, // server-side detected REQMOD request body abort
     ERR_DETAIL_ICAP_RESPMOD_EARLY, // RESPMOD failed w/o store entry
     ERR_DETAIL_ICAP_RESPMOD_LATE,  // RESPMOD failed with a store entry
     ERR_DETAIL_REQMOD_BLOCK, // REQMOD denied client access

diff --git a/src/http.cc b/src/http.cc
index 0130ef57a3cbfb2d8fdf090eec8d55213edf3607..299f53a08cbce2bcc84b026b67c05f8a1088e467 100644 (file)
--- a/src/http.cc
+++ b/src/http.cc
@@ -51,6 +51,7 @@
 #if USE_DELAY_POOLS
 #include "DelayPools.h"
 #endif
+#include "err_detail_type.h"
 #include "errorpage.h"
 #include "http.h"
 #include "HttpControlMsg.h"
@@ -2304,8 +2305,12 @@ HttpStateData::handleRequestBodyProducerAborted()
     if (entry->isEmpty()) {
         debugs(11, 3, "request body aborted: FD " << fd);
         ErrorState *err;
-        err = errorCon(ERR_READ_ERROR, HTTP_BAD_GATEWAY, fwd->request);
-        err->xerrno = errno;
+        // We usually get here when ICAP REQMOD aborts during body processing.
+        // We might also get here if client-side aborts, but then our response
+        // should not matter because either client-side will provide its own or
+        // there will be no response at all (e.g., if the the client has left).
+        err = errorCon(ERR_ICAP_FAILURE, HTTP_INTERNAL_SERVER_ERROR, fwd->request);
+        err->xerrno = ERR_DETAIL_SRV_REQMOD_REQ_BODY;
         fwd->fail(err);
     }