Skip to content

Commit b9be3ad

Browse files
fix(core): replace expect() with proper error handling in build_request
All three wire format build_request functions used .expect() which would panic on serialization failure. Now they return Result<Value, Error> and propagate errors to callers. Also adds Python unit tests for wire format validation, message building, and health check error paths. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent ea51301 commit b9be3ad

6 files changed

Lines changed: 64 additions & 18 deletions

File tree

crates/stringflow-core/src/client.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ pub async fn chat_async(
9191
messages: &[crate::ChatMessage],
9292
) -> Result<String, Error> {
9393
let url = wire_formats::endpoint(&config.base_url, config.wire_format);
94-
let body = wire_formats::build_request(messages, config);
94+
let body = wire_formats::build_request(messages, config)?;
9595

9696
let client = reqwest::Client::builder()
9797
.timeout(REQUEST_TIMEOUT)
@@ -132,7 +132,7 @@ pub async fn chat_async(
132132
/// Send a blocking chat request. Retries on 503 with exponential backoff.
133133
pub fn chat(config: &ProviderConfig, messages: &[crate::ChatMessage]) -> Result<String, Error> {
134134
let url = wire_formats::endpoint(&config.base_url, config.wire_format);
135-
let body = wire_formats::build_request(messages, config);
135+
let body = wire_formats::build_request(messages, config)?;
136136

137137
let client = reqwest::blocking::Client::builder()
138138
.timeout(REQUEST_TIMEOUT)
@@ -174,7 +174,7 @@ pub async fn chat_stream(
174174
messages: &[crate::ChatMessage],
175175
) -> Result<Pin<Box<dyn Stream<Item = Result<StreamEvent, Error>> + Send>>, Error> {
176176
let url = wire_formats::endpoint(&config.base_url, config.wire_format);
177-
let mut body = wire_formats::build_request(messages, config);
177+
let mut body = wire_formats::build_request(messages, config)?;
178178
body.as_object_mut()
179179
.ok_or_else(|| Error::RequestFailed("request body is not a JSON object".to_string()))?
180180
.insert("stream".into(), true.into());

crates/stringflow-core/src/wire_formats/completions.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,14 @@ struct CompletionsResponse {
3333
// Build / parse
3434
// ============================================================================
3535

36-
pub(crate) fn build_request(messages: &[ChatMessage], _config: &ProviderConfig) -> Value {
36+
pub(crate) fn build_request(
37+
messages: &[ChatMessage],
38+
_config: &ProviderConfig,
39+
) -> Result<Value, Error> {
3740
serde_json::to_value(CompletionsRequest {
3841
messages: messages.to_vec(),
3942
})
40-
.expect("serialize completions request")
43+
.map_err(|e| Error::RequestFailed(e.to_string()))
4144
}
4245

4346
pub(crate) fn parse_response(bytes: &[u8]) -> Result<String, Error> {
@@ -74,7 +77,7 @@ mod tests {
7477
fn request_shape() {
7578
let msgs = crate::test_messages();
7679
let config = test_config();
77-
let val = build_request(&msgs, &config);
80+
let val = build_request(&msgs, &config).unwrap();
7881
let arr = val["messages"].as_array().unwrap();
7982
assert_eq!(arr.len(), 3);
8083
assert_eq!(arr[0]["role"], "user");

crates/stringflow-core/src/wire_formats/messages.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,10 @@ struct MessagesResponse {
3535
// Build / parse
3636
// ============================================================================
3737

38-
pub(crate) fn build_request(messages: &[ChatMessage], config: &ProviderConfig) -> Value {
38+
pub(crate) fn build_request(
39+
messages: &[ChatMessage],
40+
config: &ProviderConfig,
41+
) -> Result<Value, Error> {
3942
serde_json::to_value(MessagesRequest {
4043
model: config
4144
.model
@@ -44,7 +47,7 @@ pub(crate) fn build_request(messages: &[ChatMessage], config: &ProviderConfig) -
4447
messages: messages.to_vec(),
4548
max_tokens: config.max_tokens.unwrap_or(DEFAULT_MAX_TOKENS),
4649
})
47-
.expect("serialize messages request")
50+
.map_err(|e| Error::RequestFailed(e.to_string()))
4851
}
4952

5053
pub(crate) fn parse_response(bytes: &[u8]) -> Result<String, Error> {
@@ -91,7 +94,7 @@ mod tests {
9194
fn request_shape() {
9295
let msgs = crate::test_messages();
9396
let config = test_config();
94-
let val = build_request(&msgs, &config);
97+
let val = build_request(&msgs, &config).unwrap();
9598
let arr = val["messages"].as_array().unwrap();
9699
assert_eq!(arr.len(), 3);
97100
assert!(val["model"].as_str().is_some());
@@ -105,7 +108,7 @@ mod tests {
105108
let mut config = test_config();
106109
config.model = Some("claude-opus".to_string());
107110
config.max_tokens = Some(8192);
108-
let val = build_request(&msgs, &config);
111+
let val = build_request(&msgs, &config).unwrap();
109112
assert_eq!(val["model"], "claude-opus");
110113
assert_eq!(val["max_tokens"], 8192);
111114
}

crates/stringflow-core/src/wire_formats/mod.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ pub(crate) fn endpoint(base_url: &str, format: WireFormat) -> String {
2121
format!("{}{}", base_url, path)
2222
}
2323

24-
pub(crate) fn build_request(messages: &[ChatMessage], config: &ProviderConfig) -> Value {
24+
pub(crate) fn build_request(
25+
messages: &[ChatMessage],
26+
config: &ProviderConfig,
27+
) -> Result<Value, Error> {
2528
match config.wire_format {
2629
WireFormat::Completions => completions::build_request(messages, config),
2730
WireFormat::Responses => responses::build_request(messages, config),
@@ -77,17 +80,17 @@ mod tests {
7780

7881
let mut config = test_config();
7982
config.wire_format = WireFormat::Completions;
80-
let completions = build_request(&msgs, &config);
83+
let completions = build_request(&msgs, &config).unwrap();
8184
assert!(completions.get("messages").is_some());
8285
assert!(completions.get("model").is_none());
8386

8487
config.wire_format = WireFormat::Responses;
85-
let responses = build_request(&msgs, &config);
88+
let responses = build_request(&msgs, &config).unwrap();
8689
assert!(responses.get("input").is_some());
8790
assert!(responses.get("model").is_some());
8891

8992
config.wire_format = WireFormat::Messages;
90-
let messages = build_request(&msgs, &config);
93+
let messages = build_request(&msgs, &config).unwrap();
9194
assert!(messages.get("messages").is_some());
9295
assert!(messages.get("max_tokens").is_some());
9396
}

crates/stringflow-core/src/wire_formats/responses.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,10 @@ struct ResponsesResponse {
3939
// Build / parse
4040
// ============================================================================
4141

42-
pub(crate) fn build_request(messages: &[ChatMessage], config: &ProviderConfig) -> Value {
42+
pub(crate) fn build_request(
43+
messages: &[ChatMessage],
44+
config: &ProviderConfig,
45+
) -> Result<Value, Error> {
4346
serde_json::to_value(ResponsesRequest {
4447
model: config
4548
.model
@@ -48,7 +51,7 @@ pub(crate) fn build_request(messages: &[ChatMessage], config: &ProviderConfig) -
4851
input: messages.to_vec(),
4952
max_output_tokens: config.max_tokens.unwrap_or(DEFAULT_MAX_TOKENS),
5053
})
51-
.expect("serialize responses request")
54+
.map_err(|e| Error::RequestFailed(e.to_string()))
5255
}
5356

5457
pub(crate) fn parse_response(bytes: &[u8]) -> Result<String, Error> {
@@ -90,7 +93,7 @@ mod tests {
9093
fn request_shape() {
9194
let msgs = crate::test_messages();
9295
let config = test_config();
93-
let val = build_request(&msgs, &config);
96+
let val = build_request(&msgs, &config).unwrap();
9497
let arr = val["input"].as_array().unwrap();
9598
assert_eq!(arr.len(), 3);
9699
assert_eq!(arr[0]["role"], "user");
@@ -105,7 +108,7 @@ mod tests {
105108
let mut config = test_config();
106109
config.model = Some("custom-model".to_string());
107110
config.max_tokens = Some(2048);
108-
let val = build_request(&msgs, &config);
111+
let val = build_request(&msgs, &config).unwrap();
109112
assert_eq!(val["model"], "custom-model");
110113
assert_eq!(val["max_output_tokens"], 2048);
111114
}

py/stringflow/test_api.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,33 @@ def test_connection_error_without_server(self):
3131
sf.chat("hi", base_url="http://localhost:19999")
3232

3333

34+
class TestMessageBuilding:
35+
def test_string_builds_user_message(self):
36+
"""String input should build a user message and append to history."""
37+
# chat() will fail connecting, but we can verify TypeError doesn't fire for str
38+
with pytest.raises((ConnectionError, Exception)):
39+
sf.chat("hello", base_url="http://localhost:19999")
40+
41+
def test_list_input_passes_through(self):
42+
"""List input should be used directly as messages."""
43+
with pytest.raises((ConnectionError, Exception)):
44+
sf.chat([("user", "hello")], base_url="http://localhost:19999")
45+
46+
def test_history_is_prepended(self):
47+
"""History should be prepended to the new message."""
48+
with pytest.raises((ConnectionError, Exception)):
49+
sf.chat(
50+
"follow up",
51+
[("user", "hi"), ("assistant", "hello")],
52+
base_url="http://localhost:19999",
53+
)
54+
55+
def test_invalid_wire_format(self):
56+
"""Invalid wire format should raise ValueError."""
57+
with pytest.raises(ValueError, match="unknown wire format"):
58+
sf.chat("hi", base_url="http://localhost:19999", wire_format="invalid")
59+
60+
3461
class TestDefaults:
3562
def test_default_url(self):
3663
assert sf.DEFAULT_URL == "http://localhost:8080"
@@ -42,6 +69,13 @@ def test_exports(self):
4269
assert hasattr(sf, "Message")
4370

4471

72+
class TestHealthCheck:
73+
def test_health_check_connection_error(self):
74+
"""health_check should raise when server is unreachable."""
75+
with pytest.raises((ConnectionError, Exception)):
76+
sf.health_check(base_url="http://localhost:19999")
77+
78+
4579
# ============================================================================
4680
# E2E tests (require running llama-server on localhost:8080)
4781
# ============================================================================

0 commit comments

Comments
 (0)