🤖 Found by Claude ultrareview — automated high-effort code review. Please verify independently before acting.
Location: impit/src/errors.rs:115 and :125 (ImpitError::from)
Connect-timeout and incomplete-body classification relies on substring-matching the reqwest error's Debug output:
... format!("{:?}", error).to_lowercase().contains("connection timed out") ...
... format!("{:?}", error).to_lowercase().contains("unexpectedeof") ...Impact: the Debug formatting of hyper/reqwest internal error types is not a stable API. A dependency bump can change the string, after which every connect-timeout silently downgrades to a generic TimeoutException and every truncated response to a generic error — defeating the typed error hierarchy callers rely on. The is_incomplete_message() downcast used just above shows the robust approach is available; it just isn't used consistently.
Suggested direction: classify via typed downcasts / is_* accessors on the error chain instead of stringly-matching Debug output.
Location:
impit/src/errors.rs:115and:125(ImpitError::from)Connect-timeout and incomplete-body classification relies on substring-matching the reqwest error's
Debugoutput:Impact: the
Debugformatting of hyper/reqwest internal error types is not a stable API. A dependency bump can change the string, after which every connect-timeout silently downgrades to a genericTimeoutExceptionand every truncated response to a generic error — defeating the typed error hierarchy callers rely on. Theis_incomplete_message()downcast used just above shows the robust approach is available; it just isn't used consistently.Suggested direction: classify via typed downcasts /
is_*accessors on the error chain instead of stringly-matchingDebugoutput.