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.
