fix: prevent panics in robot-mode JSON output and arithmetic paths
Peer code review found multiple panic-reachable paths: 1. serde_json::to_string().unwrap() in 4 robot-mode output functions (who.rs, main.rs x3). If serialization ever failed (e.g., NaN from edge-case division), the CLI would panic with an unhelpful stack trace. Replaced with unwrap_or_else that emits a structured JSON error fallback. 2. encode_rowid() in chunk_ids.rs used unchecked multiplication (document_id * 1000). On extreme document IDs this could silently wrap in release mode, causing embedding rowid collisions. Now uses checked_mul + checked_add with a diagnostic panic message. 3. HTTP response body truncation at byte index 500 in client.rs could split a multi-byte UTF-8 character, causing a panic. Now uses floor_char_boundary(500) for safe truncation. 4. who.rs reviews mode: SQL used `m.author_username != ?1` which silently dropped MRs with NULL author_username (SQL NULL != anything = NULL). Changed to `(m.author_username IS NULL OR m.author_username != ?1)` to match the pattern already used in expert mode. 5. handle_auth_test hardcoded exit code 5 for all errors regardless of type. Config not found (20), token not set (4), and network errors (8) all incorrectly returned 5. Now uses e.exit_code() from the actual LoreError, with proper suggestion hints in human mode. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -851,7 +851,7 @@ fn query_reviews(
|
|||||||
WHERE n.author_username = ?1
|
WHERE n.author_username = ?1
|
||||||
AND n.note_type = 'DiffNote'
|
AND n.note_type = 'DiffNote'
|
||||||
AND n.is_system = 0
|
AND n.is_system = 0
|
||||||
AND m.author_username != ?1
|
AND (m.author_username IS NULL OR m.author_username != ?1)
|
||||||
AND n.created_at >= ?2
|
AND n.created_at >= ?2
|
||||||
AND (?3 IS NULL OR n.project_id = ?3)";
|
AND (?3 IS NULL OR n.project_id = ?3)";
|
||||||
|
|
||||||
@@ -868,7 +868,7 @@ fn query_reviews(
|
|||||||
WHERE n.author_username = ?1
|
WHERE n.author_username = ?1
|
||||||
AND n.note_type = 'DiffNote'
|
AND n.note_type = 'DiffNote'
|
||||||
AND n.is_system = 0
|
AND n.is_system = 0
|
||||||
AND m.author_username != ?1
|
AND (m.author_username IS NULL OR m.author_username != ?1)
|
||||||
AND n.created_at >= ?2
|
AND n.created_at >= ?2
|
||||||
AND (?3 IS NULL OR n.project_id = ?3)";
|
AND (?3 IS NULL OR n.project_id = ?3)";
|
||||||
|
|
||||||
@@ -888,7 +888,7 @@ fn query_reviews(
|
|||||||
WHERE n.author_username = ?1
|
WHERE n.author_username = ?1
|
||||||
AND n.note_type = 'DiffNote'
|
AND n.note_type = 'DiffNote'
|
||||||
AND n.is_system = 0
|
AND n.is_system = 0
|
||||||
AND m.author_username != ?1
|
AND (m.author_username IS NULL OR m.author_username != ?1)
|
||||||
AND ltrim(n.body) LIKE '**%**%'
|
AND ltrim(n.body) LIKE '**%**%'
|
||||||
AND n.created_at >= ?2
|
AND n.created_at >= ?2
|
||||||
AND (?3 IS NULL OR n.project_id = ?3)
|
AND (?3 IS NULL OR n.project_id = ?3)
|
||||||
@@ -1798,7 +1798,12 @@ pub fn print_who_json(run: &WhoRun, args: &WhoArgs, elapsed_ms: u64) {
|
|||||||
meta: RobotMeta { elapsed_ms },
|
meta: RobotMeta { elapsed_ms },
|
||||||
};
|
};
|
||||||
|
|
||||||
println!("{}", serde_json::to_string(&output).unwrap());
|
println!(
|
||||||
|
"{}",
|
||||||
|
serde_json::to_string(&output).unwrap_or_else(|e| {
|
||||||
|
format!(r#"{{"ok":false,"error":{{"code":"INTERNAL_ERROR","message":"JSON serialization failed: {e}"}}}}"#)
|
||||||
|
})
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Serialize)]
|
#[derive(Serialize)]
|
||||||
|
|||||||
@@ -1,7 +1,12 @@
|
|||||||
pub const CHUNK_ROWID_MULTIPLIER: i64 = 1000;
|
pub const CHUNK_ROWID_MULTIPLIER: i64 = 1000;
|
||||||
|
|
||||||
pub fn encode_rowid(document_id: i64, chunk_index: i64) -> i64 {
|
pub fn encode_rowid(document_id: i64, chunk_index: i64) -> i64 {
|
||||||
document_id * CHUNK_ROWID_MULTIPLIER + chunk_index
|
document_id
|
||||||
|
.checked_mul(CHUNK_ROWID_MULTIPLIER)
|
||||||
|
.and_then(|v| v.checked_add(chunk_index))
|
||||||
|
.unwrap_or_else(|| {
|
||||||
|
panic!("encode_rowid overflow: document_id={document_id}, chunk_index={chunk_index}")
|
||||||
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn decode_rowid(rowid: i64) -> (i64, i64) {
|
pub fn decode_rowid(rowid: i64) -> (i64, i64) {
|
||||||
|
|||||||
@@ -70,15 +70,19 @@ impl GitLabClient {
|
|||||||
headers.insert(ACCEPT, HeaderValue::from_static("application/json"));
|
headers.insert(ACCEPT, HeaderValue::from_static("application/json"));
|
||||||
|
|
||||||
let client = Client::builder()
|
let client = Client::builder()
|
||||||
.default_headers(headers)
|
.default_headers(headers.clone())
|
||||||
.timeout(Duration::from_secs(30))
|
.timeout(Duration::from_secs(30))
|
||||||
.build()
|
.build()
|
||||||
.unwrap_or_else(|e| {
|
.unwrap_or_else(|e| {
|
||||||
warn!(
|
warn!(
|
||||||
error = %e,
|
error = %e,
|
||||||
"Failed to build configured HTTP client; falling back to default client"
|
"Failed to build configured HTTP client; falling back to default client with timeout"
|
||||||
);
|
);
|
||||||
Client::new()
|
Client::builder()
|
||||||
|
.default_headers(headers)
|
||||||
|
.timeout(Duration::from_secs(30))
|
||||||
|
.build()
|
||||||
|
.unwrap_or_else(|_| Client::new())
|
||||||
});
|
});
|
||||||
|
|
||||||
Self {
|
Self {
|
||||||
@@ -180,7 +184,7 @@ impl GitLabClient {
|
|||||||
let text = response.text().await?;
|
let text = response.text().await?;
|
||||||
serde_json::from_str(&text).map_err(|e| {
|
serde_json::from_str(&text).map_err(|e| {
|
||||||
let preview = if text.len() > 500 {
|
let preview = if text.len() > 500 {
|
||||||
&text[..500]
|
&text[..text.floor_char_boundary(500)]
|
||||||
} else {
|
} else {
|
||||||
&text
|
&text
|
||||||
};
|
};
|
||||||
|
|||||||
42
src/main.rs
42
src/main.rs
@@ -845,7 +845,12 @@ fn print_combined_ingest_json(
|
|||||||
meta: RobotMeta { elapsed_ms },
|
meta: RobotMeta { elapsed_ms },
|
||||||
};
|
};
|
||||||
|
|
||||||
println!("{}", serde_json::to_string(&output).unwrap());
|
println!(
|
||||||
|
"{}",
|
||||||
|
serde_json::to_string(&output).unwrap_or_else(|e| {
|
||||||
|
format!(r#"{{"ok":false,"error":{{"code":"INTERNAL_ERROR","message":"JSON serialization failed: {e}"}}}}"#)
|
||||||
|
})
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Serialize)]
|
#[derive(Serialize)]
|
||||||
@@ -874,7 +879,12 @@ fn print_combined_dry_run_json(
|
|||||||
},
|
},
|
||||||
};
|
};
|
||||||
|
|
||||||
println!("{}", serde_json::to_string(&output).unwrap());
|
println!(
|
||||||
|
"{}",
|
||||||
|
serde_json::to_string(&output).unwrap_or_else(|e| {
|
||||||
|
format!(r#"{{"ok":false,"error":{{"code":"INTERNAL_ERROR","message":"JSON serialization failed: {e}"}}}}"#)
|
||||||
|
})
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn handle_count(
|
async fn handle_count(
|
||||||
@@ -966,7 +976,12 @@ fn print_init_json(result: &InitResult) {
|
|||||||
.collect(),
|
.collect(),
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
println!("{}", serde_json::to_string(&output).unwrap());
|
println!(
|
||||||
|
"{}",
|
||||||
|
serde_json::to_string(&output).unwrap_or_else(|e| {
|
||||||
|
format!(r#"{{"ok":false,"error":{{"code":"INTERNAL_ERROR","message":"JSON serialization failed: {e}"}}}}"#)
|
||||||
|
})
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn handle_init(
|
async fn handle_init(
|
||||||
@@ -1198,17 +1213,20 @@ async fn handle_auth_test(
|
|||||||
}
|
}
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
if robot_mode {
|
if robot_mode {
|
||||||
let output = FallbackErrorOutput {
|
let output = RobotErrorOutput::from(&e);
|
||||||
error: FallbackError {
|
eprintln!(
|
||||||
code: "AUTH_FAILED".to_string(),
|
"{}",
|
||||||
message: e.to_string(),
|
serde_json::to_string(&output).unwrap_or_else(|_| {
|
||||||
},
|
format!(r#"{{"error":{{"code":"{}","message":"{}"}}}}"#, e.code(), e)
|
||||||
};
|
})
|
||||||
eprintln!("{}", serde_json::to_string(&output)?);
|
);
|
||||||
} else {
|
} else {
|
||||||
eprintln!("{}", style(format!("Error: {e}")).red());
|
eprintln!("{} {}", style("Error:").red(), e);
|
||||||
|
if let Some(suggestion) = e.suggestion() {
|
||||||
|
eprintln!("{} {}", style("Hint:").yellow(), suggestion);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
std::process::exit(5);
|
std::process::exit(e.exit_code());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user