Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions COMMIT_MESSAGE_ISSUE_377.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
fix(core/chat): surface DNS/network guidance on stream errors (#377)

- detect reqwest connect/timeout failures while starting model streams and wrap them with actionable hints
- add DNS-specific guidance so users can spot resolver misconfigurations (e.g., empty /etc/resolv.conf)
- cover hint detection with lightweight unit tests
5 changes: 5 additions & 0 deletions COMMIT_MESSAGE_ISSUE_5965.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
feat(core): include current date in environment context (#5965)

- add optional `current_date` field to `EnvironmentContext` so turn context carries ISO dates
- serialize `<current_date>` tags and extend comparisons/tests to tolerate deterministic values
- cover default date formatting with new unit tests
10 changes: 10 additions & 0 deletions PR_BODY_ISSUE_377.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
## Summary
- wrap model stream connect errors with contextual guidance so DNS/connection failures no longer surface as opaque reqwest messages
- treat timeouts and DNS resolution failures as `CodexErr::Stream` with actionable hints (e.g., check `/etc/resolv.conf`)
- add unit coverage for the DNS hint helper

## Testing
- cargo test -p code-core chat_completions::tests::dns_hint_matches_common_messages
- ./build-fast.sh

Fixes #377.
12 changes: 12 additions & 0 deletions PR_BODY_ISSUE_5965.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
## Summary
- inject the current local date into `EnvironmentContext` so every turn shares an ISO8601 `<current_date>` tag with the model
- extend the serializer/equality helpers to account for the new field while keeping comparisons deterministic in tests
- add lightweight unit coverage to lock the XML output and default date format

## Testing
- ./build-fast.sh

## Acceptance Criteria
- environment context payloads now surface a `<current_date>` element in YYYY-MM-DD form
- existing comparisons that ignore shell differences remain stable once the date is normalized
- unit tests document the new field and its formatting so regressions are caught automatically
117 changes: 116 additions & 1 deletion code-rs/core/src/chat_completions.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::collections::BTreeMap;
use std::error::Error as StdError;
use std::time::Duration;

use bytes::Bytes;
Expand Down Expand Up @@ -468,7 +469,7 @@ pub(crate) async fn stream_chat_completions(
);
let _ = logger.end_request_log(&request_id);
}
return Err(e.into());
return Err(enrich_reqwest_error(e));
}
let delay = backoff(attempt);
tokio::time::sleep(delay).await;
Expand Down Expand Up @@ -851,6 +852,120 @@ async fn process_chat_sse<S>(
}
}

fn enrich_reqwest_error(err: reqwest::Error) -> CodexErr {
let url_hint = err
.url()
.map(|u| format!(" while requesting {u}"))
.unwrap_or_default();
let root_msg = deepest_error_message(&err);
let full_msg = err.to_string();

if err.is_timeout() {
let message = format!(
"network timeout{url_hint}: {root_msg}. Please check your internet connection and try again."
);
return CodexErr::Stream(message, None);
}

if err.is_connect() {
let hint = dns_resolution_hint(&root_msg)
.or_else(|| dns_resolution_hint(&full_msg))
.unwrap_or(
"Verify that you have an active internet connection and that outbound HTTPS access to the model endpoint is allowed.",
);
let message = format!("network error{url_hint}: {root_msg}. {hint}");
return CodexErr::Stream(message, None);
}

CodexErr::Reqwest(err)
}

fn deepest_error_message(err: &dyn StdError) -> String {
let mut current: &dyn StdError = err;
let mut last = current.to_string();
while let Some(source) = current.source() {
let candidate = source.to_string();
if !candidate.is_empty() {
last = candidate;
}
current = source;
}
if last.is_empty() {
err.to_string()
} else {
last
}
}

fn dns_resolution_hint(message: &str) -> Option<&'static str> {
let lower = message.to_ascii_lowercase();
if lower.contains("dns error")
|| lower.contains("failed to lookup address information")
|| lower.contains("temporary failure in name resolution")
|| lower.contains("name or service not known")
|| lower.contains("no such host")
|| lower.contains("failed host lookup")
|| lower.contains("nodename nor servname provided")
|| lower.contains("getaddrinfo failed")
|| lower.contains("could not resolve host")
|| lower.contains("name resolution failed")
{
Some(
"Check that your DNS configuration is working (for example verify /etc/resolv.conf or your system resolver settings) and then retry.",
)
} else {
None
}
}

#[cfg(test)]
mod tests {
use super::dns_resolution_hint;
use super::deepest_error_message;
use std::error::Error as StdError;

#[test]
fn dns_hint_matches_common_messages() {
assert!(dns_resolution_hint("dns error: failed to lookup address information").is_some());
assert!(dns_resolution_hint("Temporary failure in name resolution").is_some());
assert!(dns_resolution_hint("Name or service not known").is_some());
assert!(dns_resolution_hint("No such host is known").is_some());
assert!(dns_resolution_hint("nodename nor servname provided, or not known").is_some());
assert!(dns_resolution_hint("getaddrinfo failed: Name or service not known").is_some());
}

#[test]
fn dns_hint_ignores_non_dns_errors() {
assert!(dns_resolution_hint("connection refused").is_none());
assert!(dns_resolution_hint("TLS handshake timeout").is_none());
}

#[test]
fn deepest_error_message_prefers_innermost_source() {
let err = anyhow::anyhow!("lowest cause")
.context("wrapper level 1")
.context("wrapper level 2");
let message = deepest_error_message(err.as_ref());
assert_eq!(message, "lowest cause");
}

#[test]
fn deepest_error_message_handles_empty_source_messages() {
#[derive(Debug)]
struct EmptySourceError;
impl std::fmt::Display for EmptySourceError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "")
}
}
impl StdError for EmptySourceError {}

let err = anyhow::anyhow!("top level").context(EmptySourceError);
let message = deepest_error_message(err.as_ref());
assert_eq!(message, "top level");
}
}

/// Optional client-side aggregation helper
///
/// Stream adapter that merges the incremental `OutputItemDone` chunks coming from
Expand Down
54 changes: 54 additions & 0 deletions code-rs/core/src/environment_context.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use chrono::Local;
use os_info::Type as OsType;
use os_info::Version;
use serde::Deserialize;
Expand Down Expand Up @@ -33,6 +34,7 @@ pub(crate) struct EnvironmentContext {
pub operating_system: Option<OperatingSystemInfo>,
pub common_tools: Option<Vec<String>>,
pub shell: Option<Shell>,
pub current_date: Option<String>,
}

#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
Expand Down Expand Up @@ -145,6 +147,7 @@ impl EnvironmentContext {
operating_system: detect_operating_system_info(),
common_tools: detect_common_tools(),
shell,
current_date: Some(Local::now().format("%Y-%m-%d").to_string()),
}
}

Expand All @@ -161,6 +164,7 @@ impl EnvironmentContext {
writable_roots,
operating_system,
common_tools,
current_date,
// should compare all fields except shell
shell: _,
} = other;
Expand All @@ -172,6 +176,7 @@ impl EnvironmentContext {
&& self.writable_roots == *writable_roots
&& self.operating_system == *operating_system
&& self.common_tools == *common_tools
&& self.current_date == *current_date
}
}

Expand Down Expand Up @@ -249,6 +254,9 @@ impl EnvironmentContext {
lines.push(" </common_tools>".to_string());
}
}
if let Some(current_date) = self.current_date {
lines.push(format!(" <current_date>{current_date}</current_date>"));
}
if let Some(shell) = self.shell
&& let Some(shell_name) = shell.name()
{
Expand Down Expand Up @@ -360,6 +368,7 @@ mod tests {
);
context.operating_system = None;
context.common_tools = None;
context.current_date = Some("2025-01-02".to_string());

let expected = r#"<environment_context>
<cwd>/repo</cwd>
Expand All @@ -370,6 +379,7 @@ mod tests {
<root>/repo</root>
<root>/tmp</root>
</writable_roots>
<current_date>2025-01-02</current_date>
</environment_context>"#;

assert_eq!(context.serialize_to_xml(), expected);
Expand All @@ -385,11 +395,13 @@ mod tests {
);
context.operating_system = None;
context.common_tools = None;
context.current_date = Some("2025-01-02".to_string());

let expected = r#"<environment_context>
<approval_policy>never</approval_policy>
<sandbox_mode>read-only</sandbox_mode>
<network_access>restricted</network_access>
<current_date>2025-01-02</current_date>
</environment_context>"#;

assert_eq!(context.serialize_to_xml(), expected);
Expand All @@ -405,11 +417,13 @@ mod tests {
);
context.operating_system = None;
context.common_tools = None;
context.current_date = Some("2025-01-02".to_string());

let expected = r#"<environment_context>
<approval_policy>on-failure</approval_policy>
<sandbox_mode>danger-full-access</sandbox_mode>
<network_access>enabled</network_access>
<current_date>2025-01-02</current_date>
</environment_context>"#;

assert_eq!(context.serialize_to_xml(), expected);
Expand All @@ -429,6 +443,7 @@ mod tests {
architecture: Some("aarch64".to_string()),
});
context.common_tools = Some(vec!["rg".to_string(), "git".to_string()]);
context.current_date = Some("2025-01-02".to_string());

let xml = context.serialize_to_xml();
assert!(xml.contains("<operating_system>"));
Expand All @@ -438,6 +453,7 @@ mod tests {
assert!(xml.contains("<common_tools>"));
assert!(xml.contains("<tool>rg</tool>"));
assert!(xml.contains("<tool>git</tool>"));
assert!(xml.contains("<current_date>2025-01-02</current_date>"));
}

#[test]
Expand All @@ -455,6 +471,12 @@ mod tests {
Some(workspace_write_policy(vec!["/repo"], true)),
None,
);
// ensure current_date doesn't influence this comparison
let fixed_date = Some("2025-01-02".to_string());
let mut context1 = context1;
context1.current_date = fixed_date.clone();
let mut context2 = context2;
context2.current_date = fixed_date;
assert!(!context1.equals_except_shell(&context2));
}

Expand All @@ -472,6 +494,10 @@ mod tests {
Some(SandboxPolicy::new_workspace_write_policy()),
None,
);
let mut context1 = context1;
context1.current_date = Some("2025-01-02".to_string());
let mut context2 = context2;
context2.current_date = Some("2025-01-02".to_string());

assert!(!context1.equals_except_shell(&context2));
}
Expand All @@ -490,6 +516,10 @@ mod tests {
Some(workspace_write_policy(vec!["/repo", "/tmp"], true)),
None,
);
let mut context1 = context1;
context1.current_date = Some("2025-01-02".to_string());
let mut context2 = context2;
context2.current_date = Some("2025-01-02".to_string());

assert!(!context1.equals_except_shell(&context2));
}
Expand All @@ -514,7 +544,31 @@ mod tests {
zshrc_path: "/home/user/.zshrc".into(),
})),
);
let mut context1 = context1;
context1.current_date = Some("2025-01-02".to_string());
let mut context2 = context2;
context2.current_date = Some("2025-01-02".to_string());

assert!(context1.equals_except_shell(&context2));
}

#[test]
fn serialize_environment_context_includes_current_date() {
let mut context = EnvironmentContext::new(None, None, None, None);
context.current_date = Some("2025-01-02".to_string());

let xml = context.serialize_to_xml();
assert!(xml.contains("<current_date>2025-01-02</current_date>"));
}

#[test]
fn current_date_format_is_iso8601() {
let context = EnvironmentContext::new(None, None, None, None);
let date = context
.current_date
.expect("current_date should be populated");
assert_eq!(date.len(), 10);
assert_eq!(date.chars().nth(4), Some('-'));
assert_eq!(date.chars().nth(7), Some('-'));
}
}
Loading
Loading