Skip to content

Commit 84404b4

Browse files
committed
report exceptions before closing the body
In #148 we made sure the close and flush the body so that the receiver wouldn't potentially be left waiting. This changes the order so that the error handler is always called first, and then the body eof is sent. In practice, user code will typically wait for the response handler to be called, and at that point their only signal that the request is complete is to wait for the body eof. Likewise, the signal that there was an error and the request will not complete is the error handler. You can imagine having some client setup code that looks like this: ```ocaml let result = Ivar.create () in let on_eof () = Ivar.fill_if_empty result (Ok ()) in let error_handler e = Ivar.fill_if_empty result (Error e) in (* ... *) ``` By making sure we fire the error handler first, the user can correctly identify the result of the request and not accidentally mark it as complete. Fixes #187.
1 parent a783ec0 commit 84404b4

1 file changed

Lines changed: 33 additions & 0 deletions

File tree

lib_test/test_client_connection.ml

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,38 @@ let test_report_exn () =
207207
!error_message
208208
;;
209209

210+
let test_report_exn_during_body_read () =
211+
let request' = Request.create `GET "/" in
212+
let response = Response.create `OK in
213+
214+
let body_done = ref false in
215+
let error_message = ref None in
216+
let body, t =
217+
request
218+
request'
219+
~response_handler:(fun _ body ->
220+
let on_read _ ~off:_ ~len:_ = () in
221+
let on_eof () = body_done := true in
222+
Body.schedule_read body ~on_read ~on_eof)
223+
~error_handler:(function
224+
| `Exn (Failure msg) ->
225+
Alcotest.(check bool) "body is not complete" false !body_done;
226+
error_message := Some msg
227+
| _ -> assert false)
228+
in
229+
Body.close_writer body;
230+
write_request t request';
231+
writer_closed t;
232+
reader_ready t;
233+
read_response t response;
234+
report_exn t (Failure "something went wrong");
235+
connection_is_shutdown t;
236+
Alcotest.(check (option string)) "something went wrong"
237+
(Some "something went wrong")
238+
!error_message;
239+
Alcotest.(check bool) "body is complete" true !body_done;
240+
;;
241+
210242
let test_input_shrunk () =
211243
let request' = Request.create `GET "/" in
212244
let response = Response.create `OK in (* not actually writen to the channel *)
@@ -268,6 +300,7 @@ let tests =
268300
; "Response EOF", `Quick, test_response_eof
269301
; "Response header order preserved", `Quick, test_response_header_order
270302
; "report_exn" , `Quick, test_report_exn
303+
; "report_exn_during_body_read", `Quick, test_report_exn_during_body_read
271304
; "input_shrunk", `Quick, test_input_shrunk
272305
; "failed response parse", `Quick, test_failed_response_parse
273306
]

0 commit comments

Comments
 (0)