merge: don't enter offline mode on recoverable engine errors (closes #86)

This commit is contained in:
Hunter Bown
2026-04-26 16:30:21 -05:00
2 changed files with 93 additions and 12 deletions
+32 -12
View File
@@ -639,18 +639,11 @@ async fn run_event_loop(
queued_to_send = app.pop_queued_message();
}
}
EngineEvent::Error { message, .. } => {
app.streaming_state.reset();
app.streaming_message_index = None;
app.streaming_thinking_active_entry = None;
app.add_message(HistoryCell::System {
content: format!("Error: {message}"),
});
app.offline_mode = true;
app.is_loading = false;
app.status_message = Some(format!(
"Engine error; queued messages stay pending: {message}"
));
EngineEvent::Error {
message,
recoverable,
} => {
apply_engine_error_to_app(app, message, recoverable);
}
EngineEvent::Status { message } => {
app.status_message = Some(message);
@@ -1715,6 +1708,33 @@ fn queued_session_to_ui(msg: QueuedSessionMessage) -> QueuedMessage {
}
}
/// Translate an `EngineEvent::Error` into UI state updates.
///
/// `recoverable` is the engine's own classification: stream stalls, chunk
/// timeouts, transient network errors, and rate-limit/server hiccups arrive
/// with `recoverable = true` and must NOT flip the session into offline mode
/// — the user can resend the turn and the underlying transport will retry.
/// Hard failures (auth, billing, invalid request) arrive with
/// `recoverable = false`; those flip offline mode so subsequent messages get
/// queued instead of silently lost mid-flight.
pub(crate) fn apply_engine_error_to_app(app: &mut App, message: String, recoverable: bool) {
app.streaming_state.reset();
app.streaming_message_index = None;
app.streaming_thinking_active_entry = None;
app.add_message(HistoryCell::System {
content: format!("Error: {message}"),
});
app.is_loading = false;
if recoverable {
app.status_message = Some(format!("Connection interrupted: {message}"));
} else {
app.offline_mode = true;
app.status_message = Some(format!(
"Engine error; queued messages stay pending: {message}"
));
}
}
fn persist_offline_queue_state(app: &App) {
if let Ok(manager) = SessionManager::default_location() {
if app.queued_messages.is_empty() && app.queued_draft.is_none() {
+61
View File
@@ -1914,3 +1914,64 @@ fn truncate_line_to_width_respects_display_width_for_tiny_budgets() {
assert!(trimmed_long.ends_with("..."));
assert!(UnicodeWidthStr::width(trimmed_long.as_str()) <= 10);
}
/// Regression for #86. A recoverable engine error (stream stall, transient
/// disconnect, retryable server hiccup) must NOT flip the session into
/// offline mode. Until this fix the UI matched on `EngineEvent::Error {
/// message, .. }` and unconditionally set `app.offline_mode = true`, so a
/// long V4 thinking turn whose chunked stream got closed mid-flight ended
/// the session in offline mode with the next typed message queued.
#[test]
fn recoverable_engine_error_does_not_enter_offline_mode() {
let mut app = create_test_app();
assert!(!app.offline_mode);
apply_engine_error_to_app(
&mut app,
"Stream stalled: no data received for 60s, closing stream".to_string(),
true,
);
assert!(
!app.offline_mode,
"recoverable error must keep the session online so the user can retry"
);
assert!(!app.is_loading);
let status = app
.status_message
.as_deref()
.expect("recoverable errors must set a status message");
assert!(
status.starts_with("Connection interrupted"),
"expected interrupt-style status, got {status:?}"
);
}
/// Hard failures (auth, billing, malformed request) DO need to flip offline
/// mode so subsequent typed messages get queued instead of silently lost
/// against a broken upstream.
#[test]
fn non_recoverable_engine_error_enters_offline_mode() {
let mut app = create_test_app();
assert!(!app.offline_mode);
apply_engine_error_to_app(
&mut app,
"Authentication failed: invalid API key".to_string(),
false,
);
assert!(
app.offline_mode,
"non-recoverable error must enter offline mode"
);
assert!(!app.is_loading);
let status = app
.status_message
.as_deref()
.expect("non-recoverable errors must set a status message");
assert!(
status.starts_with("Engine error"),
"expected engine-error status, got {status:?}"
);
}