Skip to content

Commit

Permalink
req_fsm: Use status 408 for reset streams
Browse files Browse the repository at this point in the history
The 503 synth and 500 minimal response status codes are too misleading
in this context, where the failure is attributed to the client. Among
existing 4XX status codes, this is the closest if we stretch the timeout
definition to "didn't complete rapidly enough before the client went
away".
  • Loading branch information
dridi committed Dec 5, 2023
1 parent 98b6b95 commit 4250083
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 6 deletions.
13 changes: 10 additions & 3 deletions bin/varnishd/cache/cache_req_fsm.c
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,13 @@ cnt_vclfail(struct worker *wrk, struct req *req)

Req_Rollback(ctx);

req->err_code = 503;
req->err_reason = "VCL failed";
if (req->req_reset) {
req->err_code = 408;
req->err_reason = "Client disconnected";
} else {
req->err_code = 503;
req->err_reason = "VCL failed";
}
req->req_step = R_STP_SYNTH;
req->doclose = SC_VCL_FAILURE;
req->filter_list = NULL;
Expand All @@ -305,6 +310,7 @@ cnt_synth(struct worker *wrk, struct req *req)
{
struct vsb *synth_body;
ssize_t sz, szl;
uint16_t status;
uint8_t *ptr;
const char *body;

Expand Down Expand Up @@ -339,7 +345,8 @@ cnt_synth(struct worker *wrk, struct req *req)
}
VSB_destroy(&synth_body);
(void)VRB_Ignore(req);
(void)req->transport->minimal_response(req, 500);
status = req->req_reset ? 408 : 500;
(void)req->transport->minimal_response(req, status);
req->doclose = SC_VCL_FAILURE; // XXX: Not necessary any more ?
VSLb_ts_req(req, "Resp", W_TIM_real(wrk));
http_Teardown(req->resp);
Expand Down
4 changes: 2 additions & 2 deletions bin/varnishtest/tests/t02025.vtc
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ varnish v1 -expect req_reset == 1
# is interpreted as before a second elapsed. Session VXIDs showing up
# numerous times become increasingly more suspicious. The format can of
# course be extended to add anything else useful for data mining.
shell -expect "1000 ${localhost}" {
shell -expect "1000 ${localhost} 408" {
varnishncsa -n ${v1_name} -d \
-q 'Timestamp:Reset[2] < 1.0' -F '%{VSL:Begin[2]}x %h'
-q 'Timestamp:Reset[2] < 1.0' -F '%{VSL:Begin[2]}x %h %s'
}
3 changes: 2 additions & 1 deletion doc/sphinx/reference/vsl.rst
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ Restart
Reset
The client closed its connection, reset its stream or caused
a stream error that forced Varnish to reset the stream. Request
processing is interrupted and considered failed.
processing is interrupted and considered failed, with a 408
"Request Timeout" status code.

Pipe handling timestamps
~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down

0 comments on commit 4250083

Please sign in to comment.