Fix UTF-8 panics and inconsistent retry logic
- Fix 7 UTF-8 byte slicing panics that crash on multi-byte characters:
- acd.rs: extract_topic_from_text() [..50] slice
- streaming.rs: log_stream_error() [..500] slice
- tools/acd.rs: rehydrate message truncation [..2000] slice
- history.rs: git commit message truncation [..69] slice
- planner.rs: commit summary/description truncation [..69] slices
- llm.rs: requirements summary line truncation [..117] slice
- All now use chars().count() and chars().take(N).collect() for
UTF-8 safe truncation
- Fix inconsistent retry logic in task_execution.rs:
- Previously only retried on Timeout errors
- Now retries on ALL recoverable errors (rate limits, network,
server errors, model busy, token limits, context length)
- Added error-specific base delays (rate limit: 5s, server: 2s, etc.)
- Added exponential backoff with ±20% jitter
- Consistent with autonomous mode retry behavior
This commit is contained in:
1
Cargo.lock
generated
1
Cargo.lock
generated
@@ -1362,6 +1362,7 @@ dependencies = [
|
|||||||
"hex",
|
"hex",
|
||||||
"indicatif",
|
"indicatif",
|
||||||
"once_cell",
|
"once_cell",
|
||||||
|
"rand",
|
||||||
"ratatui",
|
"ratatui",
|
||||||
"regex",
|
"regex",
|
||||||
"rustyline",
|
"rustyline",
|
||||||
|
|||||||
@@ -31,6 +31,7 @@ termimad = "0.34.0"
|
|||||||
regex = "1.10"
|
regex = "1.10"
|
||||||
syntect = "5.3"
|
syntect = "5.3"
|
||||||
once_cell = "1.19"
|
once_cell = "1.19"
|
||||||
|
rand = "0.8"
|
||||||
|
|
||||||
[dev-dependencies]
|
[dev-dependencies]
|
||||||
tempfile = "3.8"
|
tempfile = "3.8"
|
||||||
|
|||||||
@@ -8,10 +8,10 @@ use tracing::error;
|
|||||||
|
|
||||||
use crate::simple_output::SimpleOutput;
|
use crate::simple_output::SimpleOutput;
|
||||||
|
|
||||||
/// Maximum number of retry attempts for timeout errors
|
/// Maximum number of retry attempts for recoverable errors
|
||||||
const MAX_TIMEOUT_RETRIES: u32 = 3;
|
const MAX_RETRIES: u32 = 3;
|
||||||
|
|
||||||
/// Execute a task with retry logic for timeout errors.
|
/// Execute a task with retry logic for recoverable errors.
|
||||||
pub async fn execute_task_with_retry<W: UiWriter>(
|
pub async fn execute_task_with_retry<W: UiWriter>(
|
||||||
agent: &mut Agent<W>,
|
agent: &mut Agent<W>,
|
||||||
input: &str,
|
input: &str,
|
||||||
@@ -61,29 +61,49 @@ pub async fn execute_task_with_retry<W: UiWriter>(
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check if this is a timeout error that we should retry
|
// Check if this is a recoverable error that we should retry
|
||||||
let error_type = classify_error(&e);
|
let error_type = classify_error(&e);
|
||||||
|
|
||||||
if matches!(
|
if let ErrorType::Recoverable(recoverable_error) = error_type {
|
||||||
error_type,
|
if attempt < MAX_RETRIES {
|
||||||
ErrorType::Recoverable(RecoverableError::Timeout)
|
// Calculate retry delay with exponential backoff + jitter
|
||||||
) && attempt < MAX_TIMEOUT_RETRIES
|
let base_delay_ms = match recoverable_error {
|
||||||
{
|
RecoverableError::RateLimit => 5000, // Rate limits need longer waits
|
||||||
// Calculate retry delay with exponential backoff
|
RecoverableError::ServerError => 2000,
|
||||||
let delay_ms = 1000 * (2_u64.pow(attempt - 1));
|
RecoverableError::NetworkError => 1000,
|
||||||
|
RecoverableError::Timeout => 1000,
|
||||||
|
RecoverableError::ModelBusy => 3000,
|
||||||
|
RecoverableError::TokenLimit => 1000,
|
||||||
|
RecoverableError::ContextLengthExceeded => 1000,
|
||||||
|
};
|
||||||
|
let delay_ms = base_delay_ms * (2_u64.pow(attempt - 1));
|
||||||
|
// Add jitter (±20%)
|
||||||
|
let jitter = (delay_ms as f64 * 0.2 * (rand::random::<f64>() - 0.5)) as i64;
|
||||||
|
let delay_ms = (delay_ms as i64 + jitter).max(100) as u64;
|
||||||
let delay = std::time::Duration::from_millis(delay_ms);
|
let delay = std::time::Duration::from_millis(delay_ms);
|
||||||
|
|
||||||
|
let error_name = match recoverable_error {
|
||||||
|
RecoverableError::RateLimit => "Rate limit",
|
||||||
|
RecoverableError::ServerError => "Server error",
|
||||||
|
RecoverableError::NetworkError => "Network error",
|
||||||
|
RecoverableError::Timeout => "Timeout",
|
||||||
|
RecoverableError::ModelBusy => "Model busy",
|
||||||
|
RecoverableError::TokenLimit => "Token limit",
|
||||||
|
RecoverableError::ContextLengthExceeded => "Context length",
|
||||||
|
};
|
||||||
|
|
||||||
output.print(&format!(
|
output.print(&format!(
|
||||||
"⏱️ Timeout error detected (attempt {}/{}). Retrying in {:?}...",
|
"⚠️ {} detected (attempt {}/{}). Retrying in {:.1}s...",
|
||||||
attempt, MAX_TIMEOUT_RETRIES, delay
|
error_name, attempt, MAX_RETRIES, delay_ms as f64 / 1000.0
|
||||||
));
|
));
|
||||||
|
|
||||||
// Wait before retrying
|
// Wait before retrying
|
||||||
tokio::time::sleep(delay).await;
|
tokio::time::sleep(delay).await;
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// For non-timeout errors or after max retries
|
// For non-recoverable errors or after max retries
|
||||||
handle_execution_error(&e, input, output, attempt);
|
handle_execution_error(&e, input, output, attempt);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -335,12 +335,13 @@ fn extract_topic_from_text(text: &str) -> String {
|
|||||||
let first_line = text.lines().next().unwrap_or("");
|
let first_line = text.lines().next().unwrap_or("");
|
||||||
let cleaned = first_line.trim();
|
let cleaned = first_line.trim();
|
||||||
|
|
||||||
if cleaned.len() <= 50 {
|
if cleaned.chars().count() <= 50 {
|
||||||
cleaned.to_string()
|
cleaned.to_string()
|
||||||
} else {
|
} else {
|
||||||
// Find a good break point
|
// Find a good break point (UTF-8 safe)
|
||||||
let truncated = &cleaned[..50];
|
let truncated: String = cleaned.chars().take(50).collect();
|
||||||
if let Some(last_space) = truncated.rfind(' ') {
|
if let Some(last_space) = truncated.rfind(' ') {
|
||||||
|
// last_space is a byte index into truncated, which is safe since truncated is a new String
|
||||||
format!("{}...", &truncated[..last_space])
|
format!("{}...", &truncated[..last_space])
|
||||||
} else {
|
} else {
|
||||||
format!("{}...", truncated)
|
format!("{}...", truncated)
|
||||||
|
|||||||
@@ -200,8 +200,9 @@ pub fn log_stream_error(
|
|||||||
.rev()
|
.rev()
|
||||||
.find(|m| matches!(m.role, MessageRole::User))
|
.find(|m| matches!(m.role, MessageRole::User))
|
||||||
{
|
{
|
||||||
let truncated = if last_user_msg.content.len() > 500 {
|
let truncated = if last_user_msg.content.chars().count() > 500 {
|
||||||
format!("{}... (truncated)", &last_user_msg.content[..500])
|
let chars: String = last_user_msg.content.chars().take(500).collect();
|
||||||
|
format!("{}... (truncated)", chars)
|
||||||
} else {
|
} else {
|
||||||
last_user_msg.content.clone()
|
last_user_msg.content.clone()
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -93,8 +93,9 @@ pub async fn execute_rehydrate<W: UiWriter>(
|
|||||||
};
|
};
|
||||||
|
|
||||||
// Truncate very long messages for readability
|
// Truncate very long messages for readability
|
||||||
let content = if msg.content.len() > 2000 {
|
let content = if msg.content.chars().count() > 2000 {
|
||||||
format!("{}... [truncated, {} chars total]", &msg.content[..2000], msg.content.len())
|
let chars: String = msg.content.chars().take(2000).collect();
|
||||||
|
format!("{}... [truncated, {} chars total]", chars, msg.content.chars().count())
|
||||||
} else {
|
} else {
|
||||||
msg.content.clone()
|
msg.content.clone()
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -137,8 +137,9 @@ pub fn write_completed_requirements(
|
|||||||
pub fn write_git_commit(plan_dir: &Path, message: &str) -> Result<()> {
|
pub fn write_git_commit(plan_dir: &Path, message: &str) -> Result<()> {
|
||||||
let timestamp = format_timestamp();
|
let timestamp = format_timestamp();
|
||||||
// Truncate message if too long for a single line
|
// Truncate message if too long for a single line
|
||||||
let truncated_message = if message.len() > 72 {
|
let truncated_message = if message.chars().count() > 72 {
|
||||||
format!("{}...", &message[..69])
|
let chars: String = message.chars().take(69).collect();
|
||||||
|
format!("{}...", chars)
|
||||||
} else {
|
} else {
|
||||||
message.to_string()
|
message.to_string()
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -138,8 +138,9 @@ pub async fn generate_requirements_summary(
|
|||||||
.lines()
|
.lines()
|
||||||
.take(5)
|
.take(5)
|
||||||
.map(|line| {
|
.map(|line| {
|
||||||
if line.len() > 120 {
|
if line.chars().count() > 120 {
|
||||||
format!("{}...", &line[..117])
|
let chars: String = line.chars().take(117).collect();
|
||||||
|
format!("{}...", chars)
|
||||||
} else {
|
} else {
|
||||||
line.to_string()
|
line.to_string()
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -519,8 +519,9 @@ pub fn parse_commit_message(response: &str) -> (String, String) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Ensure summary is max 72 chars
|
// Ensure summary is max 72 chars
|
||||||
if summary.len() > 72 {
|
if summary.chars().count() > 72 {
|
||||||
summary = format!("{}...", &summary[..69]);
|
let chars: String = summary.chars().take(69).collect();
|
||||||
|
summary = format!("{}...", chars);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Ensure description lines are max 72 chars
|
// Ensure description lines are max 72 chars
|
||||||
@@ -528,8 +529,9 @@ pub fn parse_commit_message(response: &str) -> (String, String) {
|
|||||||
.lines()
|
.lines()
|
||||||
.take(10) // Max 10 lines
|
.take(10) // Max 10 lines
|
||||||
.map(|line| {
|
.map(|line| {
|
||||||
if line.len() > 72 {
|
if line.chars().count() > 72 {
|
||||||
format!("{}...", &line[..69])
|
let chars: String = line.chars().take(69).collect();
|
||||||
|
format!("{}...", chars)
|
||||||
} else {
|
} else {
|
||||||
line.to_string()
|
line.to_string()
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user