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

On 03/22/2011 06:17 PM, Amos Jeffries wrote:
> 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.

Revised the commit message and committed to trunk as r11329.

Thank you,

Alex.
Received on Wed Mar 30 2011 - 18:22:09 MDT