Repository navigation
Unified IoAction error type + HTTP response classifier #1304
Description
Problem
Each I/O component reinvents error classification:
- OTLP sink returns
SendResult(4 variants) - File tailer returns
io::Result - Receivers return
InputError(3 variants) - All 3 HTTP sinks (OTLP, Elasticsearch, Loki) duplicate identical status code → result classification (~300 lines total):
429 → RetryAfter,413 → Rejected,5xx → IoError, Retry-After header parsing
Solution
1. Unified error type
pub enum IoAction {
Retry { source: io::Error, after: Option<Duration> },
Reject { reason: String },
Fatal { source: io::Error },
}2. Shared HTTP response classifier
Extract into logfwd-output/src/http_classify.rs:
pub fn classify_http_response(response: &reqwest::Response) -> SendResult {
let status = response.status();
if status.is_success() { return SendResult::Ok; }
if status == StatusCode::TOO_MANY_REQUESTS {
let retry_after = parse_retry_after(response.headers());
return SendResult::RetryAfter(retry_after);
}
if status == StatusCode::PAYLOAD_TOO_LARGE { return SendResult::Rejected(...); }
if status.is_server_error() { return SendResult::IoError(...); }
SendResult::Rejected(...)
}Scope
- Design
IoActionenum in logfwd-types (or logfwd-output) - Extract
classify_http_response()helper - Extract
parse_retry_after()helper - Update OTLP sink to use shared classifier
- Update Elasticsearch sink to use shared classifier
- Update Loki sink to use shared classifier
Evidence
~21 instances of duplicated status code classification across 3 sinks. Deduplicating saves ~300 lines and makes adding new HTTP sinks trivial.
Relates to #1283 (I/O refactor research), #1302 (semantic fields)
Activity
github-actions commentedon Apr 6, 2026
tl;dr: Outcome B — needs human decision. I found clear duplication in HTTP status classification, but there’s a blocking semantic conflict: this issue proposes
5xx -> IoError, while open PR#1297changes OTLP5xx -> RetryAfter, and the current OTLP test suite still asserts error-on-5xx.Recommendation
Please decide one policy before implementation:
5xx -> IoError(worker uses exponential retry path)5xx -> RetryAfter(default/parsed)(worker uses server-directed retry path)I recommend Policy B because it aligns server overload handling with 429 semantics and reduces retry-policy divergence across sinks.
Please add label
needs-decisionto track this gate.Findings
SendResult(Ok | IoError | RetryAfter | Rejected):crates/logfwd-output/src/sink.rs:26RetryAftervsIoError):crates/logfwd/src/worker_pool.rs:559crates/logfwd-output/src/otlp_sink.rs:297crates/logfwd-output/src/elasticsearch.rs:419crates/logfwd-output/src/elasticsearch.rs:577crates/logfwd-output/src/loki.rs:304crates/logfwd-output/src/otlp_sink.rs:848(send_payload_returns_err_on_5xx_no_loop)InvalidInputindo_send:crates/logfwd-output/src/elasticsearch.rs:432send_batchmapsInvalidInput/InvalidDatatoRejected:crates/logfwd-output/src/elasticsearch.rs:632IoActiontype in codebase (rg "enum IoAction" crates/**/*.rs=> no match).Verification
I ran targeted existing tests in this area:
Observed result: both tests passed.
Detailed action plan (mechanical once decision is made)
crates/logfwd-output/src/http_classify.rs.parse_retry_after(headers: &reqwest::header::HeaderMap, default_secs: u64) -> Durationclassify_http_status(status: StatusCode, headers: &HeaderMap, default_retry_secs: u64) -> ...crates/logfwd-output/src/lib.rsto includemod http_classify;.crates/logfwd-output/src/otlp_sink.rs:297-330with helper.crates/logfwd-output/src/elasticsearch.rs:419-450and577-607.crates/logfwd-output/src/loki.rs:304-327.IoActionenum inlogfwd-types(orlogfwd-outputif you choose local-first rollout).SendResultat sink boundary initially; add explicit conversion helpers (IoAction <-> SendResult) to avoid worker/input churn in same PR.http_classify.rs: 200, 429(+header parse/fallback), 413, generic 4xx, 5xx.send_payload_returns_err_on_5xx_no_loop) according to decision.cargo test -p logfwd-outputcargo test -p logfwdcargo fmt --all && cargo clippy --all-targets -- -D warningsRelated items
logfwd-typesfoundational work; possible coordination for type placementcrates/logfwd-output/src/sink.rs:26SendResultcontractcrates/logfwd/src/worker_pool.rs:559crates/logfwd-output/src/otlp_sink.rs:297crates/logfwd-output/src/elasticsearch.rs:419crates/logfwd-output/src/loki.rs:304Note
🔒 Integrity filtering filtered 8 items
Integrity filtering activated and filtered the following items during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.
issue_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".)search_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".)issue_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".)issue_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".)issue_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".)pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".)pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".)pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".)What is this? | From workflow: Issue Triage
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
feat: add IoAction error classification and HTTP classifier
feat: Phase 0 I/O correctness foundation — shared types, library swap…
strawgate commentedon Apr 6, 2026
Closed-state cleanup pass on April 6, 2026: this issue was effectively resolved as part of the Phase 0 I/O refactor in PR #1334.
The body still captures the original design intent, but it should not be read as live planned work anymore. The remaining open follow-up in this area is the broader sink retry/classification cleanup tracked elsewhere, not this specific foundational extraction step.