Skip to content

Commit f5d22b9

Browse files
authored
fix: persist path-attach .gitkeep blobs and stop panicking on missing objects (#2152)
GitHub sync / import attach created trees that referenced a placeholder .gitkeep but never wrote it to object storage, so clone/fetch hit S3 404 and upload-pack unwrap turned that into HTTP 502. Save the blob on attach and map pack failures to protocol errors instead.
1 parent 4a68770 commit f5d22b9

8 files changed

Lines changed: 205 additions & 211 deletions

File tree

‎Cargo.lock‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎ceres/src/application/api_service/mono/sync.rs‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ impl MonoApiService {
4646
let expected_tree = root_ref.ref_tree_hash.clone();
4747
let root_ref_id = root_ref.id;
4848

49-
let save_trees = tree_ops::search_and_create_tree(self, &path_buf).await?;
49+
let (save_trees, gitkeep_blob) =
50+
tree_ops::search_and_create_tree(self, &path_buf).await?;
5051
let leaf_tree = save_trees
5152
.back()
5253
.ok_or_else(|| MegaError::Other("no tree generated".to_string()))?;
@@ -60,6 +61,13 @@ impl MonoApiService {
6061
&commit_msg,
6162
);
6263

64+
// Persist .gitkeep before attaching trees. Previously only trees/commits
65+
// were saved, leaving object storage without the blob and breaking clone.
66+
self.storage()
67+
.mono_service
68+
.save_blobs(&new_commit.id.to_string(), vec![gitkeep_blob])
69+
.await?;
70+
6371
let txn = self.storage().begin_db_transaction().await?;
6472
match storage
6573
.attach_to_monorepo_parent_in_txn(

‎ceres/src/application/api_service/tree_ops.rs‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use git_internal::{
99
errors::GitError,
1010
internal::object::{
1111
ObjectTrait,
12+
blob::Blob,
1213
tree::{Tree, TreeItem, TreeItemMode},
1314
},
1415
};
@@ -112,11 +113,15 @@ pub async fn search_tree_by_path<T: ApiHandler + ?Sized>(
112113
///
113114
/// # Errors
114115
///
115-
/// Returns a `GitError` if an error occurs during the search or tree creation process.
116+
/// Returns a `MegaError` if an error occurs during the search or tree creation process.
117+
///
118+
/// The returned [`Blob`] is the placeholder `.gitkeep` referenced by the new leaf tree.
119+
/// Callers **must** persist it via object storage (`save_blobs` / `put_objects`) before
120+
/// or with the trees; otherwise clone/fetch will 404 on that object.
116121
pub async fn search_and_create_tree<T: ApiHandler + ?Sized>(
117122
handler: &T,
118123
path: &Path,
119-
) -> Result<VecDeque<Tree>, MegaError> {
124+
) -> Result<(VecDeque<Tree>, Blob), MegaError> {
120125
let relative_path = handler.strip_relative(path)?;
121126
let root_tree = handler.get_root_tree(None).await?;
122127
let mut search_tree = root_tree.clone();
@@ -190,7 +195,7 @@ pub async fn search_and_create_tree<T: ApiHandler + ?Sized>(
190195
}
191196
}
192197

193-
Ok(saving_trees)
198+
Ok((saving_trees, blob))
194199
}
195200

196201
/// return the dir's hash only

‎ceres/src/application/code_edit/post_receive/import.rs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ pub async fn dispatch_import_receive_pack_finalized(
6363
let expected_tree = root_ref.ref_tree_hash.clone();
6464
let root_ref_id = root_ref.id;
6565

66-
let save_trees = tree_ops::search_and_create_tree(mono_api_service, &repo_path).await?;
66+
let (save_trees, gitkeep_blob) =
67+
tree_ops::search_and_create_tree(mono_api_service, &repo_path).await?;
6768

6869
let new_commit = Commit::from_tree_id(
6970
save_trees
@@ -74,6 +75,12 @@ pub async fn dispatch_import_receive_pack_finalized(
7475
&format!("\n{commit_msg}"),
7576
);
7677

78+
// Persist placeholder .gitkeep referenced by newly created path trees.
79+
storage
80+
.mono_service
81+
.save_blobs(&new_commit.id.to_string(), vec![gitkeep_blob])
82+
.await?;
83+
7784
let txn = storage.begin_db_transaction().await?;
7885
let git_db = storage.git_db_storage();
7986
for cmd in &commands {

‎ceres/src/transport/protocol/smart.rs‎

Lines changed: 41 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use std::{
66
use anyhow::Result;
77
use bytes::{Buf, BufMut, Bytes, BytesMut};
88
use callisto::sea_orm_active_enums::RefTypeEnum;
9-
use common::errors::{ProtocolError, mega_to_protocol_error};
9+
use common::errors::{ProtocolError, git_to_protocol_error, mega_to_protocol_error};
1010
use tokio_stream::wrappers::ReceiverStream;
1111

1212
use crate::{
@@ -153,48 +153,53 @@ impl SmartSession {
153153
let have: Vec<String> = have.into_iter().collect();
154154

155155
if have.is_empty() {
156-
pack_data = repo_handler.full_pack(want).await.unwrap();
156+
pack_data = repo_handler.full_pack(want).await.map_err(|e| {
157+
tracing::error!(error = %e, "git upload-pack full_pack failed");
158+
git_to_protocol_error(e)
159+
})?;
157160
add_pkt_line_string(&mut protocol_buf, String::from("NAK\n"));
158-
} else {
159-
if self.capabilities.contains(&Capability::MultiAckDetailed) {
160-
// multi_ack_detailed mode, the server will differentiate the ACKs where it is signaling that
161-
// it is ready to send data with ACK obj-id ready lines,
162-
// and signals the identified common commits with ACK obj-id common lines
163-
164-
for hash in &have {
165-
if repo_handler.check_commit_exist(hash).await {
166-
add_pkt_line_string(&mut protocol_buf, format!("ACK {hash} common\n"));
167-
if last_common_commit.is_empty() {
168-
last_common_commit = hash.to_string();
169-
}
161+
} else if self.capabilities.contains(&Capability::MultiAckDetailed) {
162+
// multi_ack_detailed mode, the server will differentiate the ACKs where it is signaling that
163+
// it is ready to send data with ACK obj-id ready lines,
164+
// and signals the identified common commits with ACK obj-id common lines
165+
166+
for hash in &have {
167+
if repo_handler.check_commit_exist(hash).await {
168+
add_pkt_line_string(&mut protocol_buf, format!("ACK {hash} common\n"));
169+
if last_common_commit.is_empty() {
170+
last_common_commit = hash.to_string();
170171
}
171172
}
172-
pack_data = repo_handler
173-
.incremental_pack(want.clone(), have)
174-
.await
175-
.unwrap();
176-
177-
if last_common_commit.is_empty() {
178-
//send NAK if missing common commit
179-
add_pkt_line_string(&mut protocol_buf, String::from("NAK\n"));
180-
// need to handle rebase option, still need pack data when has no common commit
181-
return Ok((pack_data, protocol_buf));
182-
}
173+
}
174+
pack_data = repo_handler
175+
.incremental_pack(want.clone(), have)
176+
.await
177+
.map_err(|e| {
178+
tracing::error!(error = %e, "git upload-pack incremental_pack failed");
179+
git_to_protocol_error(e)
180+
})?;
181+
182+
if last_common_commit.is_empty() {
183+
//send NAK if missing common commit
184+
add_pkt_line_string(&mut protocol_buf, String::from("NAK\n"));
185+
// need to handle rebase option, still need pack data when has no common commit
186+
return Ok((pack_data, protocol_buf));
187+
}
183188

184-
for hash in want {
185-
if self.capabilities.contains(&Capability::NoDone) {
186-
// If multi_ack_detailed and no-done are both present, then the sender is free to immediately send a pack
187-
// following its first "ACK obj-id ready" message.
188-
add_pkt_line_string(&mut protocol_buf, format!("ACK {hash} ready\n"));
189-
}
189+
for hash in want {
190+
if self.capabilities.contains(&Capability::NoDone) {
191+
// If multi_ack_detailed and no-done are both present, then the sender is free to immediately send a pack
192+
// following its first "ACK obj-id ready" message.
193+
add_pkt_line_string(&mut protocol_buf, format!("ACK {hash} ready\n"));
190194
}
191-
} else {
192-
tracing::error!("capability unsupported");
193-
// init a empty receiverstream
194-
let (_, rx) = tokio::sync::mpsc::channel::<Vec<u8>>(1);
195-
pack_data = ReceiverStream::new(rx);
196195
}
197196
add_pkt_line_string(&mut protocol_buf, format!("ACK {last_common_commit} \n"));
197+
} else {
198+
tracing::error!("capability unsupported");
199+
// init a empty receiverstream
200+
let (_, rx) = tokio::sync::mpsc::channel::<Vec<u8>>(1);
201+
pack_data = ReceiverStream::new(rx);
202+
add_pkt_line_string(&mut protocol_buf, format!("ACK {last_common_commit} \n"));
198203
}
199204
Ok((pack_data, protocol_buf))
200205
}

‎common/src/errors.rs‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,16 +352,36 @@ pub fn protocol_error_is_client_safe(err: &ProtocolError) -> bool {
352352
pub fn mega_to_protocol_error(err: MegaError) -> ProtocolError {
353353
match err {
354354
MegaError::NotFound(msg) => ProtocolError::NotFound(msg),
355+
MegaError::ObjStorageNotFound(msg) => {
356+
// Missing blob/object content: return a typed client error instead of
357+
// panicking in upload-pack (which surfaces to git as HTTP 502).
358+
ProtocolError::NotFound(format!("git object missing from object storage: {msg}"))
359+
}
360+
MegaError::ObjStorageInconsistent(msg) => {
361+
ProtocolError::InvalidInput(format!("git object storage inconsistent: {msg}"))
362+
}
355363
MegaError::BadRequest(msg) => ProtocolError::InvalidInput(msg),
356364
MegaError::Unauthorized(msg) => ProtocolError::Deny(msg),
357365
MegaError::Forbidden(msg) => ProtocolError::InvalidInput(msg),
358366
MegaError::Unavailable(msg) => ProtocolError::InvalidInput(msg),
359367
MegaError::Conflict(msg) => ProtocolError::InvalidInput(msg),
368+
MegaError::Git(e) => git_to_protocol_error(e),
360369
MegaError::Io(e) => ProtocolError::IO(e),
361370
other => ProtocolError::InvalidInput(other.to_string()),
362371
}
363372
}
364373

374+
/// Map [`GitError`] into a protocol-layer error for upload/receive-pack handlers.
375+
pub fn git_to_protocol_error(err: GitError) -> ProtocolError {
376+
let msg = err.to_string();
377+
match git_error_http_status(&err) {
378+
404 => ProtocolError::NotFound(msg),
379+
401 => ProtocolError::Deny(msg),
380+
400 | 403 | 409 | 413 => ProtocolError::InvalidInput(msg),
381+
_ => ProtocolError::IO(std::io::Error::other(msg)),
382+
}
383+
}
384+
365385
#[cfg(test)]
366386
mod tests {
367387
use super::*;
@@ -414,4 +434,20 @@ mod tests {
414434
let err = mega_to_protocol_error(MegaError::NotFound("repo".into()));
415435
assert!(matches!(err, ProtocolError::NotFound(_)));
416436
}
437+
438+
#[test]
439+
fn mega_to_protocol_error_maps_objstorage_not_found() {
440+
let err = mega_to_protocol_error(MegaError::ObjStorageNotFound("missing key".into()));
441+
assert!(matches!(err, ProtocolError::NotFound(_)));
442+
assert_eq!(protocol_error_http_status(&err), 404);
443+
}
444+
445+
#[test]
446+
fn git_to_protocol_error_maps_objstorage_custom_error() {
447+
let err = git_to_protocol_error(GitError::CustomError(
448+
"ObjStorage not found: Object at location git/ab/cd/ef/123 not found".into(),
449+
));
450+
assert!(matches!(err, ProtocolError::NotFound(_)));
451+
assert_eq!(protocol_error_http_status(&err), 404);
452+
}
417453
}

‎orion/Cargo.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "orion"
3-
version = "0.1.2"
3+
version = "0.1.3"
44
edition = "2024"
55

66
[[bin]]

0 commit comments

Comments
 (0)