Error handling from filter modules

[Maxim Dounin]

Hello!

On Fri, Oct 16, 2015 at 06:15:30PM +0100, Steven Hartland wrote:

> On 16/10/2015 13:20, Maxim Dounin wrote:
> >Hello!
> >
> >On Fri, Oct 16, 2015 at 02:36:13AM +0100, Steven Hartland wrote:
> >
> >>I'm making changes to a filter module and when it detected an error it
> >>returned NGX_ERROR however the response generated to the client isn't
> >>the expected 500 internal server error I would have expected given said
> >>return.
> >>
> >>So the question is do filters have to manually call
> >>ngx_http_finalize_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR); or is it
> >>expected that the upper layers should actually do the right thing and
> >>ensure the client doesn't get a bad response generated from the current
> >>state of r with no indication an error occurred?
> >In filters, it's already to late to return anything in case of
> >errors.  In body filters, it's way too late - the response was
> >already partially sent.  And in header filters there is a chance that
> >other filters allocated something response-specific, and an
> >attempt to return a different response will break things.  So,
> >when you return NGX_ERROR from a filter, the connection is just
> >closed.
> There are a few core filters which seem to contradict this;
> 
> range does:
> return ngx_http_range_not_satisfiable(r, ctx);
> 
> while image does:
> return ngx_http_filter_finalize_request(r,
> &ngx_http_image_filter_module,
> NGX_HTTP_UNSUPPORTED_MEDIA_TYPE);

There is no contradiction here.  These are special cases which use 
ngx_http_filter_finalize_request(), as outlined later in the same 
message.

> The range filter is the one I was looking at, picking back up the work you
> gave feedback on a while back with regards allow partial content responses
> to be used to satisfy range requests.
> 
> During testing for failures I triggered the NGX_ERROR case here:
> https://github.com/nginx/nginx/blob/master/src/http/modules/ngx_http_range_filter_module.c#L233
> 
> When I triggered this case client recieved an broken 200 response, where as
> if I did finalise the request I could deliver a 500 response, so was
> wondering if this would be correct in this situation?

When the NGX_ERROR case is triggered, no response is returned, the 
connection is just closed.  As this expected to happen if and only 
if we weren't able to allocate memory, this is believed to be 
perfectly correct behaviour.  Trying to do anything else is very 
unlikely to succeed, but likely to result in additional problems.

-- 
Maxim Dounin
http://nginx.org/