basho/webmachine — Issue #60: Errors in stream body functions when using webmachine_error_handler cause re-evaluation
Author: beerriot
Created: 2012-02-28 03:09:36+00:00
State: open

When using the `webmachine_error_handler` module as the configured `error_handler`, encountering an error during the evaulation of a stream-producing function will cause Webmachine to call the original stream function a second time.

The `webmachine_error_handler:render_error/3` function checks the result of `webmachine_request:has_response_body/0`, and then returns that value if it exists. The problem is that by the time this is called in the situation described above, a call to `webmachine_request:set_resp_body/1` has already been made, with the stream-producer as the argument. So, the "body" that is now to be sent as the error response is, in fact, the "body" that just produced the error.

It's also a little bit of a problem that an error response is sent at all in this case, since it breaks the chunked encoding that was already being sent, but one thing at a time…

As an example, take the following resource, which produces one byte per second for 3 seconds, then raises an `undef` error:

```erlang

-module(logdis_resource).
-export([init/1, to_html/2]).

-include_lib("webmachine/include/webmachine.hrl").

init([]) -> {ok, undefined}.

to_html(ReqData, State) ->
{{stream, {<<"a">>, fun() -> slow_response(10) end}}, ReqData, State}.

slow_response(0) ->
{<<>>, done};
slow_response(7) ->
this:doesnt(exist);
slow_response(N) ->
io:format("~p~n", [N]),
timer:sleep(1000),
{<<"a">>, fun() -> slow_response(N-1) end }.
```

The response you will see begins with a normal 200 OK block of headers, followed by 4 chunks, then a 500 Internal Server Error block of headers, followed by the same 4 chunks, then another 500, this time followed by the expected descriptive HTML message.

What's happening is that the catch in `webmachine_decision_core:handle_request/2` catches the first time the body producer bombs. This sends execution to `webmachine_decision_core:error_response/1,2`, where the call to `webmachine_error_handler:render_error/3` is made. This function returns the same `{stream, _}` tuple we started with (because it was the argument to `webmachine_request:set_resp_body/1` in `webmachine_decision_core:decision(v3018)`). Execution then moves to `respond/1` again, where the stream is evaluated again, under the assumption that it's the error message that should be sent. This time the error pops us out to the inner catch of `webmachine_mochiweb:loop/1`, which calls its own `handle_error/3` function. This function also calls `webmachine_error_handler:render_error/3`, but it passes a version of the request that was not modified by the resource or `webmachine_decision_core`, and therefore does not already have a body set, so we end up with generic error HTML.

The best workaround at the moment is likely either to modify `webmachine_error_handler` to check if the body that is set is a stream producer, and ignore it if it is, on the assumption that error messages are generally not stream based; or perhaps to clear the the set response body in the catch clause of `webmachine_decision_core:handle_request/2`, on the assumption that a resource function would have used `{halt, Code}` to send a specific error, and not `throw(…)`.