mirror of
https://github.com/fawney19/Aether.git
synced 2026-10-06 01:17:46 +08:00
fix(gateway): settle stream attempts dropped before first byte
A local stream attempt writes its `usage` row and its `request_candidates` slot as `pending` in `execute_execution_runtime_stream_inner`, then awaits the provider's response headers. Everything after that point runs inside the downstream request future, so a client disconnect drops it: the dispatch `.await` never resumes and nothing settles either row. The stream finalizer that already covers this only exists once upstream headers have arrived, so the pre-first-byte window has no owner at all. Both rows stay `pending` until the maintenance sweeper rewrites them as a 504 timeout ten minutes later, losing the real outcome, the real latency, and the 499. `AttemptCancellationGuard` takes that window. It is created disarmed, so an attempt dropped before it owns any row does not grow a settlement row it never had; it is armed as soon as the attempt owns its non-terminal rows, and the stream wrappers disarm it the moment the attempt returns, from where settlement belongs to the transport. On a cancelling drop it settles the candidate slot through the same snapshot writer the `pending` write above it uses, and the usage row through a terminal `Cancelled` event. The guard outlives the request future, so what it captures is retained for the whole attempt. It therefore holds no request body: the plan carries the provider request body and the report context carries the client request body, and keeping both would double the request-body residency of every in-flight stream attempt to serve a path that almost never runs. Simply omitting them is not safe either, because a terminal write is body-capture-authoritative: with both absent the seed carries the typed `none` marker, which clears the stored capture rather than leaving it alone. `build_usage_event_data_seed_describing_request_bodies` is the third option -- it derives every capture state, body reference, request type and derived request fact from the real plan and report context, and leaves out only the two body values -- so the guard's snapshot is small and its terminal write preserves the capture the `pending` write recorded. The stream candidate first-byte watchdog also drops the attempt future, but it settles the attempt itself through `build_transport_error_stop_response`. It now marks the attempt abandoned before returning so the guard stands down instead of racing a 499 against the watchdog's 504. Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
committed by
ZheFox
co-authored by
Claude Opus 5
parent
14744abd57
commit
9282cce1d6
@@ -14,6 +14,9 @@ use aether_data::repository::provider_catalog::InMemoryProviderCatalogReadReposi
|
||||
use aether_data_contracts::repository::candidate_selection::{
|
||||
StoredMinimalCandidateSelectionRow, StoredProviderModelMapping,
|
||||
};
|
||||
use aether_data_contracts::repository::candidates::{
|
||||
RequestCandidateReadRepository, RequestCandidateStatus,
|
||||
};
|
||||
use aether_data_contracts::repository::provider_catalog::{
|
||||
StoredProviderCatalogEndpoint, StoredProviderCatalogKey, StoredProviderCatalogProvider,
|
||||
};
|
||||
@@ -427,6 +430,112 @@ async fn gateway_stops_execution_runtime_stream_when_client_disconnects_impl() {
|
||||
upstream_handle.abort();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn gateway_settles_stream_attempt_when_client_disconnects_before_first_byte() {
|
||||
run_lifecycle_test(
|
||||
"gateway_settles_stream_attempt_when_client_disconnects_before_first_byte",
|
||||
gateway_settles_stream_attempt_when_client_disconnects_before_first_byte_impl,
|
||||
);
|
||||
}
|
||||
|
||||
async fn gateway_settles_stream_attempt_when_client_disconnects_before_first_byte_impl() {
|
||||
// The execution runtime accepts the plan and then goes quiet, so the attempt
|
||||
// is parked between its `pending` rows and the first upstream byte.
|
||||
let execution_runtime = Router::new().route(
|
||||
"/v1/execute/stream",
|
||||
any(|_request: Request| async move {
|
||||
tokio::time::sleep(std::time::Duration::from_secs(30)).await;
|
||||
StatusCode::OK
|
||||
}),
|
||||
);
|
||||
|
||||
let (execution_runtime_url, execution_runtime_handle) = start_server(execution_runtime).await;
|
||||
let auth_repository = Arc::new(InMemoryAuthApiKeySnapshotRepository::seed(vec![(
|
||||
Some(hash_api_key("sk-client-openai-stream-precommit-disconnect")),
|
||||
sample_local_openai_auth_snapshot(
|
||||
"api-key-openai-lifecycle-local-1",
|
||||
"user-openai-lifecycle-local-1",
|
||||
),
|
||||
)]));
|
||||
let candidate_selection_repository =
|
||||
Arc::new(InMemoryMinimalCandidateSelectionReadRepository::seed(vec![
|
||||
sample_local_openai_candidate_row(),
|
||||
]));
|
||||
let provider_catalog_repository = Arc::new(InMemoryProviderCatalogReadRepository::seed(
|
||||
vec![sample_local_openai_provider()],
|
||||
vec![sample_local_openai_endpoint()],
|
||||
vec![sample_local_openai_key()],
|
||||
));
|
||||
let request_candidate_repository = Arc::new(InMemoryRequestCandidateRepository::default());
|
||||
let gateway = build_router_with_state(
|
||||
build_state_with_execution_runtime_override(execution_runtime_url)
|
||||
.with_data_state_for_tests(
|
||||
GatewayDataState::with_auth_candidate_selection_provider_catalog_and_request_candidate_repository_for_tests(
|
||||
auth_repository,
|
||||
candidate_selection_repository,
|
||||
provider_catalog_repository,
|
||||
Arc::clone(&request_candidate_repository),
|
||||
DEVELOPMENT_ENCRYPTION_KEY,
|
||||
),
|
||||
),
|
||||
);
|
||||
let (gateway_url, gateway_handle) = start_server(gateway).await;
|
||||
|
||||
let request = reqwest::Client::new()
|
||||
.post(format!("{gateway_url}/v1/chat/completions"))
|
||||
.header(http::header::CONTENT_TYPE, "application/json")
|
||||
.header(
|
||||
http::header::AUTHORIZATION,
|
||||
"Bearer sk-client-openai-stream-precommit-disconnect",
|
||||
)
|
||||
.header(
|
||||
TRACE_ID_HEADER,
|
||||
"trace-openai-chat-stream-precommit-disconnect-123",
|
||||
)
|
||||
.body("{\"model\":\"gpt-5\",\"messages\":[],\"stream\":true}")
|
||||
.send();
|
||||
|
||||
// Drop the in-flight request the way a downstream client does when its own
|
||||
// first-byte timeout fires, before any response header exists.
|
||||
assert!(
|
||||
tokio::time::timeout(std::time::Duration::from_millis(750), request)
|
||||
.await
|
||||
.is_err(),
|
||||
"the execution runtime should not have answered before the client gave up"
|
||||
);
|
||||
|
||||
let mut stored_candidates = Vec::new();
|
||||
for _ in 0..200 {
|
||||
stored_candidates = request_candidate_repository
|
||||
.list_by_request_id("trace-openai-chat-stream-precommit-disconnect-123")
|
||||
.await
|
||||
.expect("request candidate trace should read");
|
||||
if stored_candidates
|
||||
.iter()
|
||||
.any(|candidate| candidate.status == RequestCandidateStatus::Cancelled)
|
||||
{
|
||||
break;
|
||||
}
|
||||
tokio::time::sleep(std::time::Duration::from_millis(10)).await;
|
||||
}
|
||||
|
||||
let cancelled = stored_candidates
|
||||
.iter()
|
||||
.find(|candidate| candidate.status == RequestCandidateStatus::Cancelled)
|
||||
.unwrap_or_else(|| {
|
||||
panic!("dropped stream attempt should settle as cancelled: {stored_candidates:?}")
|
||||
});
|
||||
assert_eq!(cancelled.status_code, Some(499));
|
||||
assert_eq!(
|
||||
cancelled.error_type.as_deref(),
|
||||
Some("local_stream_attempt_cancelled")
|
||||
);
|
||||
assert!(cancelled.finished_at_unix_ms.is_some());
|
||||
|
||||
gateway_handle.abort();
|
||||
execution_runtime_handle.abort();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn gateway_returns_error_body_when_prefetch_detects_embedded_stream_error() {
|
||||
run_lifecycle_test(
|
||||
|
||||
Reference in New Issue
Block a user