Skip to content

Commit

Permalink
ByteStreamServer now responds with no-data-received instead of NotFound
Browse files Browse the repository at this point in the history
If a client tries to upload a file, but the first payload fails, the
client may call query_write_status(), which we now return "0" (no data
received yet) instead of NotFound to increase compatibility with
clients.
  • Loading branch information
allada committed Sep 9, 2024
1 parent f337391 commit 2d5a574
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 8 deletions.
9 changes: 8 additions & 1 deletion nativelink-service/src/bytestream_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,14 @@ impl ByteStreamServer {

let has_fut = store_clone.has(digest);
let Some(item_size) = has_fut.await.err_tip(|| "Failed to call .has() on store")? else {
return Err(make_err!(Code::NotFound, "{}", "not found"));
// We lie here and say that the stream needs to start over, even though
// it was never started. This can happen when the client disconnects
// before sending the first payload, but the client thinks it did send
// the payload.
return Ok(Response::new(QueryWriteStatusResponse {
committed_size: 0,
complete: false,
}));
};
Ok(Response::new(QueryWriteStatusResponse {
committed_size: item_size as i64,
Expand Down
16 changes: 9 additions & 7 deletions nativelink-service/tests/bytestream_server_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -866,19 +866,21 @@ pub async fn test_query_write_status_smoke_test() -> Result<(), Box<dyn std::err
let resource_name = make_resource_name(raw_data.len());

{
// If the write does not exist it should respond with a 0 size.
// This is because the client may have tried to write, but the
// connection closed before the payload had been received.
let response = bs_server
.query_write_status(Request::new(QueryWriteStatusRequest {
resource_name: resource_name.clone(),
}))
.await;
assert_eq!(response.is_err(), true);
let expected_err = make_err!(
Code::NotFound,
"{}{}",
"status: NotFound, message: \"not found : Failed on query_write_status() ",
"command\", details: [], metadata: MetadataMap { headers: {} }"
assert_eq!(
response.unwrap().into_inner(),
QueryWriteStatusResponse {
committed_size: 0,
complete: false,
}
);
assert_eq!(Into::<Error>::into(response.unwrap_err()), expected_err);
}

// Setup stream.
Expand Down

0 comments on commit 2d5a574

Please sign in to comment.