No error raised when timeout occurs reading response body

[AndrewCarterUK]

Current Behaviour

If a timeout occurs before the client has received the full response body, no error is triggered despite the fact that the length of the returned body does not match the Content-Length header.

Expected Behaviour

If a timeout occurs before the client has received the full response body, an error condition should be raised.

Possible Fix

In client_connection.ml:

  let set_error_and_handle t error =
    Reader.force_close t.reader;
    begin match !(t.state) with
    | Closed -> ()
    | Awaiting_response ->
      set_error_and_handle_without_shutdown t error;
    | Received_response(_, response_body) ->
      Body.close_reader response_body;
      Body.execute_read response_body;
      set_error_and_handle_without_shutdown t error;
    end
  ;;

The order of instructions here is that the response body is closed before the error condition is raised. This triggered a handler for EOF before the error is set.

Setting the error first, fixes the issue in my testing:

  let set_error_and_handle t error =
    Reader.force_close t.reader;
    begin match !(t.state) with
    | Closed -> ()
    | Awaiting_response ->
      set_error_and_handle_without_shutdown t error;
    | Received_response(_, response_body) ->
      set_error_and_handle_without_shutdown t error;
      Body.close_reader response_body;
      Body.execute_read response_body;
    end
  ;;

[dpatti]

To be clear, the "timeout" here came from a runtime, though in principal it can be substituted with any arbitrary Client_connection.report_exn.