Skip to content

fix: Record a client that leaves before the head as client_canceled #1777

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
huang195 merged 1 commit into from
Nov 11, 2026
Merged

fix: Record a client that leaves before the head as client_canceled #1777

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion praxis/filters/src/filters/observe.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ impl HttpFilter for CortexObserve {
}
o.done = true;
// Before the response head, Praxis 0.8.0 gives a client that went
// away as status 499. After the head, it gives the written status and
// away as status 499, or as 0 when it wrote nothing. After the head, it gives the written status and

@clawgenti clawgenti Nov 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This line is now 109 columns and the rest of the block wraps near 80. rustfmt does not wrap comments, so it is worth rewrapping by hand.

// `upstream_reached` for both a client and an upstream that went away.
// shortcut: a stream cut after the head is always `client_canceled`
// (the frequent case: a user stop or a Cortex idle cut), so an
Expand Down
19 changes: 18 additions & 1 deletion praxis/filters/src/record/row.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,9 @@ impl Exchange {
None => format!("request body over the {MAX_REQUEST_BODY}-byte buffer limit"),
};
(413, ("proxy_error", String::new(), message))
} else if downstream_gone || status == 499 {
// Status 0: Praxis wrote nothing to the client, so the client left
// before any answer (an upstream failure is written as 502).
} else if downstream_gone || status == 499 || status == 0 {
(499, ("client_canceled", code, CLIENT_GONE.to_owned()))
} else if upstream_gone {
(
Expand Down Expand Up @@ -1138,6 +1140,21 @@ mod tests {
}
}

// Seen in the #1725 live check: the agent exited while the hop waited for
// the head, so Praxis wrote nothing and gave status 0.
#[test]
fn deferred_nothing_written_is_client_canceled() {

@clawgenti clawgenti Nov 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: This unit test pins deferred(0, false, false). It does not pin the fact that Praxis actually reports status 0 here. That comes from with_body.rs in praxis-proxy-protocol 0.8.0, and the same assumption already broke once: the old comment said "always 499". A gateway test in server/tests/gateway.rs would catch a Praxis bump that changes it again. It fits the existing harness. Sketch (it passed on head and failed on base when I ran it):

#[test]
fn client_leaves_before_head_is_client_canceled() {
    let o = origin(|_, s| {
        std::thread::sleep(Duration::from_millis(1500));
        respond(s, "200 OK", &[("Content-Type", "text/plain")], b"ok");
    });
    let i = ingest();
    let g = gateway(&o, &i, true);
    let mut s = g.connect();
    s.write_all(&head("GET", "/v1/models", &[("X-Cortex-Token", TOKEN)], 0))
        .expect("head");
    std::thread::sleep(Duration::from_millis(300));
    let _ = s.shutdown(Shutdown::Both);
    let lines = i.take(2);
    let resp = response_row(&lines);
    assert_eq!(resp["event"]["statusCode"], 499);
    assert_eq!(resp["event"]["error"]["kind"], "client_canceled");
}

let f = fixture();
let mut ex = exchange("GET", "api.example.com", &f.target, &f.req_headers);
let rows: Vec<Value> = ex.deferred(0, false, false).iter().map(json_of).collect();
assert_eq!(rows.len(), 2);
assert_eq!(rows[1]["event"]["statusCode"], 499);
assert_eq!(
rows[1]["event"]["error"],
json!({"kind": "client_canceled", "message": CLIENT_GONE})
);
}

#[test]
fn deferred_upstream_gone_is_upstream_error() {
let rows = streaming_exchange().deferred(200, false, true);
Expand Down
Loading