Show the endpoint's error message, not the envelope around it
Every endpoint siGit talks to reports failures in the OpenAI error envelope, and siGit Code Cloud writes those messages for the person reading them. We handed the status line and the raw body straight to the client, which the editor puts in its error banner, so hitting the monthly allowance read as: endpoint returned 429 Too Many Requests: {"error":{"message":"Monthly siGit Code Cloud allowance reached. It resets at the start of your next billing period.","type":"server_error"}} describe_api_error now unwraps the envelope and returns the message on its own. The status is still there when there is nothing to unwrap, and a body with no envelope is truncated at 500 bytes so an HTML error page from a proxy does not fill the banner. The prompt handler stops prefixing "inference failed:" for the same reason; that context is in the log line next to it. Second half of the same problem: an endpoint that fails after the response is already open reports it as a data: frame in the stream rather than as a status. The frame carries no choices, so it parsed as an empty chunk and was skipped, and the turn ended looking like the model had answered with nothing. Both the end-to-end tests fail on the old code, the streaming one with an end_turn and no error at all. Closes getsigit/sigit-si#53.
paydii committed
Sep 5, 2026 at 01:27 UTC
6d7ad1d1722a681140838b4e178993ecacda49ae
5 files changed
+419
-5
.agents/AGENTS.md
+7
-1
@@ -116,7 +116,13 @@ feeds results back. Neither the loop nor ACP/TUI surfaces depend on a concrete b
116
smbCloud, and stay general otherwise).
117
- **`src/backend.rs`** — the `InferenceBackend` trait and neutral types (`ToolSpec`, `ToolCall`,
118
`ToolResult`, `TurnResult`). Two impls: `LocalBackend` (on-device via `onde::ChatEngine`) and
119
- `OpenAiBackend` (any OpenAI-compatible HTTP endpoint).
119
+ `OpenAiBackend` (any OpenAI-compatible HTTP endpoint). A `BackendError` is user-facing: the
120
+ ACP prompt handler passes it to the client verbatim and the editor puts it in an error banner,
121
+ so `describe_api_error` unwraps the OpenAI `{"error":{"message":…}}` envelope and shows that
122
+ message on its own, falling back to the status only when there is nothing to unwrap. An
123
+ endpoint can also fail *after* the response is open, reporting it as a `data:` frame holding
124
+ the same envelope; that frame has no `choices`, so `consume_stream` has to check for it
125
+ explicitly or it parses as an empty chunk and the turn ends looking like an empty answer.
126
- **`src/provider.rs`** — decides *which* backend serves inference. Resolution order, first match
127
wins: (1) override via `OPENAI_BASE_URL`+`OPENAI_API_KEY` or active profile in
128
`~/.config/sigit/providers.toml`; (2) siGit Code Cloud when logged in; (3) on-device.
CHANGELOG.md
+15
@@ -1,5 +1,20 @@
1
# Changelog
2
3
+## Unreleased
4
+
5
+### Fixed
6
+
7
+- **A refused turn now shows the endpoint's own message.** siGit pasted the
8
+ status line and the raw JSON body into the editor's error banner, so a
9
+ billing or allowance message arrived wrapped in
10
+ `endpoint returned 429 Too Many Requests: {"error":{"message":…`. The message
11
+ the endpoint wrote is what the banner shows now; the status is only used when
12
+ there is no message to show
13
+- **An error that arrives mid-stream is no longer silent.** An endpoint that
14
+ fails after the response is already open reports it as a frame in the stream.
15
+ That frame carries no `choices`, so siGit parsed it as an empty chunk and
16
+ skipped it, and the turn ended as though the model had answered with nothing
17
+
18
## 1.5.6
19
20
### What changed
src/backend.rs
+126
-2
@@ -478,8 +478,8 @@ impl OpenAiBackend {
478
479
if !response.status().is_success() {
480
let status = response.status();
481
- let detail = response.text().await.unwrap_or_default();
482
- return Err(format!("endpoint returned {status}: {detail}"));
481
+ let body = response.text().await.unwrap_or_default();
482
+ return Err(describe_api_error(status, &body));
483
}
484
485
if let Some(sink) = sink {
@@ -560,6 +560,16 @@ impl OpenAiBackend {
560
continue;
561
}
562
563
+ // An endpoint that fails after the status line is already sent
564
+ // reports it in the stream instead, as an ordinary `data:`
565
+ // frame holding an error envelope. siGit Code Cloud does this
566
+ // when the upstream fails mid-turn. It has no `choices`, so
567
+ // without this it parses as an empty chunk and is skipped, and
568
+ // the turn ends looking like the model simply said nothing.
569
+ if let Some(message) = api_error_message(data) {
570
+ return Err(message);
571
+ }
572
+
573
let chunk: StreamCompletion = match serde_json::from_str(data) {
574
Ok(chunk) => chunk,
575
// Skip keep-alive comments and anything we can't parse rather
@@ -778,6 +788,67 @@ impl InferenceBackend for OpenAiBackend {
788
}
789
}
790
791
+// ── OpenAI error shape ────────────────────────────────────────────────────────
792
+
793
+/// The OpenAI error envelope: `{"error": {"message": ..., "type": ...}}`.
794
+///
795
+/// Every endpoint siGit talks to speaks it, including siGit Code Cloud, whose
796
+/// messages are written for the person reading them ("Monthly siGit Code Cloud
797
+/// allowance reached..."). Worth unwrapping rather than pasting the raw body
798
+/// into the editor's error banner.
799
+#[derive(Debug, Deserialize)]
800
+struct ApiErrorEnvelope {
801
+ error: ApiErrorBody,
802
+}
803
+
804
+#[derive(Debug, Deserialize)]
805
+struct ApiErrorBody {
806
+ #[serde(default)]
807
+ message: Option<String>,
808
+}
809
+
810
+/// How much of an unparseable error body to keep. An endpoint behind a proxy
811
+/// can answer with a full HTML page, and the whole thing ends up in the
812
+/// editor's error banner.
813
+const ERROR_BODY_LIMIT: usize = 500;
814
+
815
+/// Turn an error response into something worth showing a person.
816
+///
817
+/// The endpoint's own message wins when there is one: it is written for the
818
+/// user, and the status code repeats what it already says. Anything else falls
819
+/// back to the status plus whatever the body held, which is all there is to go
820
+/// on.
821
+fn describe_api_error(status: reqwest::StatusCode, body: &str) -> String {
822
+ if let Some(message) = api_error_message(body) {
823
+ return message;
824
+ }
825
+
826
+ let body = body.trim();
827
+ if body.is_empty() {
828
+ return format!("endpoint returned {status}");
829
+ }
830
+
831
+ let mut detail = body;
832
+ if detail.len() > ERROR_BODY_LIMIT {
833
+ let mut cut = ERROR_BODY_LIMIT;
834
+ while !detail.is_char_boundary(cut) {
835
+ cut -= 1;
836
+ }
837
+ detail = &detail[..cut];
838
+ return format!("endpoint returned {status}: {detail}…");
839
+ }
840
+ format!("endpoint returned {status}: {detail}")
841
+}
842
+
843
+/// The human-readable message out of an OpenAI error envelope, if the body is
844
+/// one and carries a non-empty message.
845
+fn api_error_message(body: &str) -> Option<String> {
846
+ let envelope: ApiErrorEnvelope = serde_json::from_str(body).ok()?;
847
+ let message = envelope.error.message?;
848
+ let message = message.trim();
849
+ (!message.is_empty()).then(|| message.to_string())
850
+}
851
+
852
// ── OpenAI response shapes ────────────────────────────────────────────────────────
853
854
#[derive(Debug, Deserialize)]
@@ -967,6 +1038,59 @@ pub async fn adopt_carryover(backend: &dyn InferenceBackend, carried: Vec<serde_
1038
mod tests {
1039
use super::*;
1040
1041
+ /// The case from the issue: the endpoint says something a person can act
1042
+ /// on, and the status code and JSON around it are noise.
1043
+ #[test]
1044
+ fn an_endpoints_own_message_is_what_the_user_sees() {
1045
+ let body = r#"{"error":{"message":"Monthly siGit Code Cloud allowance reached. It resets at the start of your next billing period.","type":"server_error"}}"#;
1046
+
1047
+ let described = describe_api_error(reqwest::StatusCode::TOO_MANY_REQUESTS, body);
1048
+
1049
+ assert_eq!(
1050
+ described,
1051
+ "Monthly siGit Code Cloud allowance reached. \
1052
+ It resets at the start of your next billing period."
1053
+ );
1054
+ }
1055
+
1056
+ #[test]
1057
+ fn a_body_that_is_not_an_error_envelope_keeps_the_status() {
1058
+ let described =
1059
+ describe_api_error(reqwest::StatusCode::BAD_GATEWAY, "<html>bad gateway</html>");
1060
+
1061
+ assert!(described.contains("502"), "{described}");
1062
+ assert!(described.contains("bad gateway"), "{described}");
1063
+ }
1064
+
1065
+ #[test]
1066
+ fn an_empty_body_still_says_something() {
1067
+ let described = describe_api_error(reqwest::StatusCode::INTERNAL_SERVER_ERROR, " ");
1068
+
1069
+ assert_eq!(described, "endpoint returned 500 Internal Server Error");
1070
+ }
1071
+
1072
+ #[test]
1073
+ fn an_oversized_body_is_cut_down() {
1074
+ let body = "x".repeat(ERROR_BODY_LIMIT * 3);
1075
+
1076
+ let described = describe_api_error(reqwest::StatusCode::BAD_GATEWAY, &body);
1077
+
1078
+ assert!(described.len() < ERROR_BODY_LIMIT + 100, "{described}");
1079
+ assert!(described.ends_with('…'), "{described}");
1080
+ }
1081
+
1082
+ /// An envelope with nothing useful in it must not shadow the status, which
1083
+ /// would leave the user with an empty error.
1084
+ #[test]
1085
+ fn an_envelope_with_a_blank_message_falls_back() {
1086
+ let described = describe_api_error(
1087
+ reqwest::StatusCode::FORBIDDEN,
1088
+ r#"{"error":{"message":" "}}"#,
1089
+ );
1090
+
1091
+ assert!(described.contains("403"), "{described}");
1092
+ }
1093
+
1094
#[test]
1095
fn tools_json_wraps_function_schema() {
1096
let tools = vec![ToolSpec {
src/main.rs
+9
-2
@@ -1536,7 +1536,11 @@ impl SiGitAgent {
1536
.await
1537
.map_err(|error| {
1538
log::error!("send_message_with_tools failed: {error}");
1539
- agent_client_protocol::Error::new(-32603, format!("inference failed: {error}"))
1539
+ // Verbatim, no prefix: the backend has already turned this into
1540
+ // something worth reading (see `describe_api_error`), and it is
1541
+ // what the editor puts in its error banner. The context a
1542
+ // prefix would add is in the log line above.
1543
+ agent_client_protocol::Error::new(-32603, error)
1544
})?;
1545
1546
let mut round = 0;
@@ -1734,7 +1738,10 @@ impl SiGitAgent {
1738
&mut reply,
1739
)
1740
.await
1737
- .map_err(|e| agent_client_protocol::Error::new(-32603, e.to_string()))?;
1741
+ .map_err(|error| {
1742
+ log::error!("send_tool_results failed: {error}");
1743
+ agent_client_protocol::Error::new(-32603, error)
1744
+ })?;
1745
}
1746
1747
// ── Final text response ───────────────────────────────────────────
tests/acp_endpoint_errors.rs
new
+262
@@ -0,0 +1,262 @@
1
+//! What the editor shows when the endpoint refuses a turn.
2
+//!
3
+//! Every endpoint siGit talks to reports failures in the OpenAI error envelope,
4
+//! and siGit Code Cloud writes those messages for the person reading them
5
+//! ("Monthly siGit Code Cloud allowance reached…"). The agent used to hand the
6
+//! raw status line and JSON body to the client, so Zed's error banner showed
7
+//! the envelope rather than the sentence inside it.
8
+//!
9
+//! Two shapes to cover: a failing HTTP status, and an endpoint that only
10
+//! discovers the problem after the status line is out and reports it as a
11
+//! `data:` frame in the stream.
12
+
13
+use std::collections::VecDeque;
14
+use std::io::{BufRead, BufReader, Read, Write};
15
+use std::net::TcpListener;
16
+use std::process::{Child, ChildStdin, Command, Stdio};
17
+use std::sync::Mutex;
18
+use std::sync::mpsc::{Receiver, channel};
19
+use std::time::{Duration, Instant};
20
+
21
+use serde_json::{Value, json};
22
+
23
+const TIMEOUT: Duration = Duration::from_secs(30);
24
+
25
+const ALLOWANCE_MESSAGE: &str = "Monthly siGit Code Cloud allowance reached. It resets at the start of your next billing period.";
26
+
27
+/// One scripted reply per request: an HTTP status plus a body.
28
+struct Reply {
29
+ status: &'static str,
30
+ content_type: &'static str,
31
+ body: String,
32
+}
33
+
34
+fn error_status_reply() -> Reply {
35
+ Reply {
36
+ status: "429 Too Many Requests",
37
+ content_type: "application/json",
38
+ body: json!({"error": {"message": ALLOWANCE_MESSAGE, "type": "server_error"}}).to_string(),
39
+ }
40
+}
41
+
42
+/// A 200 that turns into an error partway through, the way an endpoint has to
43
+/// report a failure it only sees once the response is already open.
44
+fn error_in_stream_reply() -> Reply {
45
+ let frame = json!({"error": {"message": ALLOWANCE_MESSAGE, "type": "server_error"}});
46
+ Reply {
47
+ status: "200 OK",
48
+ content_type: "text/event-stream",
49
+ body: format!("data: {frame}\n\n"),
50
+ }
51
+}
52
+
53
+fn start_fake_endpoint(replies: Vec<Reply>) -> u16 {
54
+ let listener = TcpListener::bind("127.0.0.1:0").expect("bind fake endpoint");
55
+ let port = listener.local_addr().unwrap().port();
56
+ let queue = Mutex::new(VecDeque::from(replies));
57
+
58
+ std::thread::spawn(move || {
59
+ for stream in listener.incoming() {
60
+ let Ok(mut stream) = stream else { continue };
61
+ let mut reader = BufReader::new(match stream.try_clone() {
62
+ Ok(clone) => clone,
63
+ Err(_) => continue,
64
+ });
65
+ let mut content_length = 0usize;
66
+ loop {
67
+ let mut line = String::new();
68
+ if reader.read_line(&mut line).unwrap_or(0) == 0 {
69
+ break;
70
+ }
71
+ let line = line.trim();
72
+ if line.is_empty() {
73
+ break;
74
+ }
75
+ if let Some(length) = line.to_ascii_lowercase().strip_prefix("content-length:") {
76
+ content_length = length.trim().parse().unwrap_or(0);
77
+ }
78
+ }
79
+ let mut body = vec![0u8; content_length];
80
+ if reader.read_exact(&mut body).is_err() {
81
+ continue;
82
+ }
83
+ let reply = queue.lock().unwrap().pop_front().unwrap_or_else(|| Reply {
84
+ status: "200 OK",
85
+ content_type: "text/event-stream",
86
+ body: "data: [DONE]\n\n".to_string(),
87
+ });
88
+ let response = format!(
89
+ "HTTP/1.1 {}\r\ncontent-type: {}\r\ncontent-length: {}\r\n\
90
+ connection: close\r\n\r\n{}",
91
+ reply.status,
92
+ reply.content_type,
93
+ reply.body.len(),
94
+ reply.body
95
+ );
96
+ let _ = stream.write_all(response.as_bytes());
97
+ let _ = stream.flush();
98
+ }
99
+ });
100
+
101
+ port
102
+}
103
+
104
+struct AgentUnderTest {
105
+ child: Child,
106
+ stdin: ChildStdin,
107
+ incoming: Receiver<Value>,
108
+ next_id: u64,
109
+}
110
+
111
+fn spawn_agent(port: u16, config_dir: &std::path::Path) -> AgentUnderTest {
112
+ let mut child = Command::new(env!("CARGO_BIN_EXE_sigit"))
113
+ .env("OPENAI_BASE_URL", format!("http://127.0.0.1:{port}"))
114
+ .env("OPENAI_API_KEY", "test-key")
115
+ .env("SIGIT_MODEL", "scripted-model")
116
+ .env("SIGIT_CONFIG_DIR", config_dir)
117
+ .env("SIGIT_MCP", "off")
118
+ .env("SIGIT_PERMISSIONS", "allow")
119
+ .env_remove("SIGIT_LOCAL_INFERENCE")
120
+ .stdin(Stdio::piped())
121
+ .stdout(Stdio::piped())
122
+ .stderr(Stdio::null())
123
+ .spawn()
124
+ .expect("spawn sigit in ACP mode");
125
+
126
+ let stdout = child.stdout.take().unwrap();
127
+ let (message_tx, incoming) = channel();
128
+ std::thread::spawn(move || {
129
+ for line in BufReader::new(stdout).lines() {
130
+ let Ok(line) = line else { break };
131
+ if let Ok(message) = serde_json::from_str::<Value>(&line)
132
+ && message_tx.send(message).is_err()
133
+ {
134
+ break;
135
+ }
136
+ }
137
+ });
138
+
139
+ let stdin = child.stdin.take().unwrap();
140
+ AgentUnderTest {
141
+ child,
142
+ stdin,
143
+ incoming,
144
+ next_id: 0,
145
+ }
146
+}
147
+
148
+impl AgentUnderTest {
149
+ fn request(&mut self, method: &str, params: Value) -> u64 {
150
+ self.next_id += 1;
151
+ let id = self.next_id;
152
+ let mut line =
153
+ json!({"jsonrpc": "2.0", "id": id, "method": method, "params": params}).to_string();
154
+ line.push('\n');
155
+ self.stdin.write_all(line.as_bytes()).expect("write stdin");
156
+ self.stdin.flush().expect("flush stdin");
157
+ id
158
+ }
159
+
160
+ fn wait_for_message(&mut self, id: u64) -> Value {
161
+ let deadline = Instant::now() + TIMEOUT;
162
+ loop {
163
+ let remaining = deadline.saturating_duration_since(Instant::now());
164
+ let Ok(message) = self.incoming.recv_timeout(remaining) else {
165
+ panic!("timed out waiting for the response to request {id}");
166
+ };
167
+ if message["id"] == id && message.get("method").is_none() {
168
+ return message;
169
+ }
170
+ }
171
+ }
172
+
173
+ fn wait_for_response(&mut self, id: u64) -> Value {
174
+ let message = self.wait_for_message(id);
175
+ assert!(
176
+ message.get("error").is_none(),
177
+ "request {id} failed: {message}"
178
+ );
179
+ message
180
+ }
181
+
182
+ fn open_session(&mut self, cwd: &std::path::Path) -> String {
183
+ let id = self.request(
184
+ "initialize",
185
+ json!({"protocolVersion": 1, "clientCapabilities": {}}),
186
+ );
187
+ self.wait_for_response(id);
188
+
189
+ let id = self.request("session/new", json!({"cwd": cwd, "mcpServers": []}));
190
+ self.wait_for_response(id)["result"]["sessionId"]
191
+ .as_str()
192
+ .expect("session id")
193
+ .to_string()
194
+ }
195
+}
196
+
197
+impl Drop for AgentUnderTest {
198
+ fn drop(&mut self) {
199
+ let _ = self.child.kill();
200
+ let _ = self.child.wait();
201
+ }
202
+}
203
+
204
+fn scratch(name: &str) -> std::path::PathBuf {
205
+ let dir = std::env::temp_dir().join(format!("sigit_acp_err_{name}_{}", std::process::id()));
206
+ std::fs::create_dir_all(dir.join("config")).unwrap();
207
+ std::fs::create_dir_all(dir.join("work")).unwrap();
208
+ dir
209
+}
210
+
211
+/// The message the endpoint wrote is the message the client gets: no status
212
+/// line in front of it, no JSON envelope around it.
213
+fn assert_is_the_endpoints_message(error: &Value) {
214
+ let message = error["error"]["message"].as_str().unwrap_or_default();
215
+ assert_eq!(message, ALLOWANCE_MESSAGE, "full error: {error}");
216
+}
217
+
218
+#[test]
219
+fn a_refused_turn_reports_the_endpoints_own_message() {
220
+ let dir = scratch("status");
221
+ let port = start_fake_endpoint(vec![error_status_reply()]);
222
+ let mut agent = spawn_agent(port, &dir.join("config"));
223
+ let session_id = agent.open_session(&dir.join("work"));
224
+
225
+ let id = agent.request(
226
+ "session/prompt",
227
+ json!({"sessionId": session_id, "prompt": [{"type": "text", "text": "hello"}]}),
228
+ );
229
+ let response = agent.wait_for_message(id);
230
+
231
+ assert!(
232
+ response.get("error").is_some(),
233
+ "expected a failure: {response}"
234
+ );
235
+ assert_is_the_endpoints_message(&response);
236
+
237
+ std::fs::remove_dir_all(&dir).ok();
238
+}
239
+
240
+/// An error that arrives as a stream frame used to parse as a chunk with no
241
+/// choices and get skipped, so the turn ended as if the model had said nothing.
242
+#[test]
243
+fn an_error_frame_mid_stream_is_not_swallowed() {
244
+ let dir = scratch("stream");
245
+ let port = start_fake_endpoint(vec![error_in_stream_reply()]);
246
+ let mut agent = spawn_agent(port, &dir.join("config"));
247
+ let session_id = agent.open_session(&dir.join("work"));
248
+
249
+ let id = agent.request(
250
+ "session/prompt",
251
+ json!({"sessionId": session_id, "prompt": [{"type": "text", "text": "hello"}]}),
252
+ );
253
+ let response = agent.wait_for_message(id);
254
+
255
+ assert!(
256
+ response.get("error").is_some(),
257
+ "expected a failure: {response}"
258
+ );
259
+ assert_is_the_endpoints_message(&response);
260
+
261
+ std::fs::remove_dir_all(&dir).ok();
262
+}