feat: install OpenAI error response formatter for proxy failures - #825
feat: install OpenAI error response formatter for proxy failures#825skamenan7 wants to merge 5 commits into
Conversation
f89aa40 to
dbbe250
Compare
|
Missing Signed-off-by: dbbe250. All commits require sign-off (via |
praxis-bot
left a comment
There was a problem hiding this comment.
PR Review
Summary: Installs OpenAI error response formatter for positively classified OpenAI requests (Responses and Chat Completions).
Overall: Core implementation is clean with good JSON escaping via serde_json. Two medium-severity issues: incompatible error types for 4xx codes and missing Unicode test coverage.
| Severity | Count |
|---|---|
| Critical | 0 |
| Large | 0 |
| Medium | 2 |
| let error_type = if context.status >= 500 { | ||
| "server_error" | ||
| } else { | ||
| context.code |
There was a problem hiding this comment.
[Medium] Incompatible error types for 4xx status codes
The error type mapping for non-5xx codes uses Praxis-internal error codes (context.code) rather than OpenAI-compatible error types. OpenAI SDKs expect standardized types:
invalid_request_error(400, 422)authentication_error(401)permission_error(403)not_found_error(404)rate_limit_error(429)server_error(5xx)
Current behavior for 429 with Praxis code "upstream_rate_limited":
{"error": {"type": "upstream_rate_limited", "code": "upstream_rate_limited"}}But OpenAI SDKs expect "type": "rate_limit_error" for retry logic and error classification.
Fix with status-based mapping:
fn map_error_type(status: u16) -> &'static str {
match status {
500..=599 => "server_error",
429 => "rate_limit_error",
401 => "authentication_error",
403 => "permission_error",
404 => "not_found_error",
400 | 422 => "invalid_request_error",
_ => "api_error",
}
}
let error_type = map_error_type(context.status);| } | ||
|
|
||
| #[test] | ||
| fn json_escaping_handles_special_characters() { |
There was a problem hiding this comment.
[Medium] Missing Unicode/non-ASCII test coverage
The test suite verifies JSON escaping for ASCII special characters but doesn't test Unicode or non-ASCII characters in error messages. While serde_json handles Unicode correctly, this is a security-sensitive JSON output path that should have explicit coverage.
Scenarios not covered:
- Multi-byte UTF-8:
"Connection to 日本 failed" - Emoji:
"Rate limit exceeded 🚫" - Right-to-left text
Add test:
#[test]
fn json_escaping_handles_unicode() {
let ctx = ErrorResponseContext::new(
"server_error",
"Connection to サーバー failed 🔥",
500
);
let response = OpenAiErrorFormatter.format(&ctx);
let parsed: serde_json::Value = serde_json::from_slice(&response.body).unwrap();
assert_eq!(
parsed["error"]["message"].as_str().unwrap(),
"Connection to サーバー failed 🔥"
);
// Verify valid UTF-8
assert!(std::str::from_utf8(&response.body).is_ok());
}|
@skamenan7 please rebase |
… failures
Add an ErrorResponseFormatter implementation that produces the standard
OpenAI {"error": {...}} JSON envelope when Praxis synthesizes a fatal
proxy response (connection refusal, timeout, etc.).
The formatter is installed as a request extension after positive OpenAI
classification (Responses, Chat Completions, and bodyless subresources).
Praxis invokes it from fail_to_proxy instead of emitting RFC 9457
Problem Details.
Mapping:
- error.message ← context.message
- error.code ← context.code (Praxis machine-readable code)
- error.type ← "server_error" for 5xx, context.code otherwise
- error.param ← always null
- Content-Type: application/json
Non-OpenAI classifications (Anthropic, unknown, invalid, non-JSON) do
not install the formatter and continue to receive the generic fallback.
Closes: RHAIENG-6940
Ref: praxis-proxy#682
Signed-off-by: Sumanth Kamenani <skamenan@redhat.com>
Signed-off-by: Sumanth Kamenani <skamenan@redhat.com>
Signed-off-by: Sumanth Kamenani <skamenan@redhat.com>
…rage Signed-off-by: Sumanth Kamenani <skamenan@redhat.com>
dbbe250 to
a9080eb
Compare
praxis-bot
left a comment
There was a problem hiding this comment.
Re-review (4 new commits since prior review)
Both prior findings have been addressed:
map_error_type()now maps HTTP status codes to OpenAI-standard error types (rate_limit_error,authentication_error, etc.) with test coverage for all mapped codes.- Unicode/non-ASCII test coverage added (
json_escaping_handles_unicodecovering Japanese, emoji, and Arabic characters with UTF-8 validation).
| Severity | Count |
|---|---|
| Critical | 0 |
| Large | 1 |
| Medium | 0 |
Findings
[Large] Missing integration test for error formatting end-to-end behavior.
The PR adds a new capability (OpenAI error response formatting on proxy failures) but only includes unit tests. The PR checklist explicitly marks "New capabilities include an example config and functional example test" as unchecked. CONTRIBUTING.md requires integration tests for new capabilities.
Add an integration test that configures a proxy with an unreachable upstream, sends an OpenAI-classified request (e.g. POST /v1/responses with a valid JSON body), and verifies:
- Response body is
{"error": {"message": "...", "type": "server_error", "param": null, "code": "..."}} Content-Typeisapplication/json- Status code is the appropriate 5xx
An example config is not applicable since this behavior is auto-installed through the existing openai_responses_format filter, not a separate config entry.
Signed-off-by: Sumanth Kamenani <skamenan@redhat.com>
|
@skamenan7 please address the bots review |
|
Thanks for manual validation, this is almost good to go! |
Summary
Installs the
OpenAiErrorFormatteron requests positively classified as OpenAI formats (Responses or Chat Completions). When a fatal proxy failure occurs (e.g., upstream connection refused or timeout), this ensures downstream clients receive a standard OpenAI-compatible{"error": {...}}JSON envelope rather than a default proxy error response.This is a focused change covering the formatter installation and unit testing.
Related issue
Closes #682
Validation
make lintStarted
praxis-ai-proxylocally with a configuration mapping/v1/responsesto a non-existent upstream (127.0.0.1:3001).Sent a request to trigger a connection refusal:
curl -v -s -X POST http://127.0.0.1:8080/v1/responses \ -H "Content-Type: application/json" \ -d '{"model": "gpt-4", "input": "Hello"}'The proxy intercepted the
fail_to_proxyerror and formatted it as a valid OpenAI error response:Checklist
Signed-off-bytrailer.Breaking changes
None.