Re: [PATCH] Better reporting of REQMOD failures during body processing

 On Tue, 22 Mar 2011 13:02:03 -0600, Alex Rousskov wrote:
> Better reporting of REQMOD failures during body processing.
>
> When REQMOD body processing fails, the server side needs to abort the
> in-progress transaction with the origin server. Use a new custom
> error
> detail and HTTP 500 (Internal Server Error) instead of 502 (Bad
> Gateway)
> status code for this case.
>
> The HTTP 500 (Internal Server Error) is more appropriate because most
> deployments consider adaptation to be a "part of the proxy service"
> rather than an "external gateway". And the custom error detail
> carries
> more information than a potentially stale and often irrelevant errno
> global.
>

 IMHO, we need to help dispell that marketing fud. 'service' is a
 technical term and applies to Squid in a specific software-daemon way.
 Trying to apply marketing level definitions on HTTP protocol values is
 plain wrong.

 "external" is the wrong word anyway. The definition says 502 is "
    The server, while acting as a gateway or proxy, received an invalid
    response from the upstream server it accessed in attempting to
    fulfill the request.
 ".
 So, we come down to: is an "ICAP server" a *server*? (yes), "upstream"?
 (um, not for REQMOD), and is it being used to "fulfill the request"?
 (yes)

 If you do commit this with 500 please remove/change the last paragraph
 of the comments.
  "most deployments" also use 302 in response to POST while saying they
 are HTTP/1.1.

 This would be better:
 "
   The HTTP 500 (Internal Server Error) is more appropriate because it
   is unclear whether 502 applies to HTTP auxiliary servers like ICAP.

   The custom error detail carries more information than a potentially
   stale and often irrelevant errno global.
 "

 NP: we have the same conundrum when getting a non-502 reply from
 cache_peers when they have done the mangling. They are not the upstream
 either, and apparently the upstream presented them with a valid reply
 (non-502).

>
> Earlier Squid versions (e.g., v3.0) were returning HTTP 500 (Internal
> Server Error) error under similar conditions. Then, after some code
> improvements, we started handing these errors better, but the new
> code
> used ERR_READ_ERROR and the 502 (Bad Gateway) codes for the symmetry
> with similar error handling code.
>
> Unfortunately, preserving that symmetry was wrong in this case
> because
> we are not dealing with a server-side reading error here. After an
> upgrade to v3.2, some users noticed the increased number of 502 (Bad
> Gateway) errors, which wrongly implied origin server problems rather
> than ICAP server problems.

 +1. Fine, with the documentation change.

 Did you have plans for an ERR_DETAIL_* when REQMOD aborts during
 headers? There will be EARLY and LATE variants of this internally as
 well. Depending on whether the body bytes sent are retry-able.

 Amos
Received on Wed Mar 23 2011 - 00:17:28 MDT