Skip to content

Timeout classification inconsistencies in classify_submission_error #10

Description

@BRBussy

Description

There is a logic inconsistency in how network timeouts are classified between typed error handling and string-pattern matching in the transaction submission error classification.

File: app/solana/cmd/api/src/api/transaction/v1/service_impl.rs

Current Behavior

Typed error examination (lines 130-138) — correctly returns Indeterminate:

// Network transport errors - connectivity, timeouts, HTTP issues (INDETERMINATE)
ClientErrorKind::Io(_) => SubmissionResult::Indeterminate,

ClientErrorKind::Reqwest(reqwest_error) => {
    if reqwest_error.is_timeout() {
        // Timeouts are especially dangerous - transaction might have been sent
    }
    // Connection/request failures - also indeterminate
    SubmissionResult::Indeterminate
}

String-pattern matching in classify_by_message (lines 286-289) — incorrectly returns FailedNetworkError:

} else if error_str.contains("network")
    || error_str.contains("connection")
    || error_str.contains("timeout")
{
    SubmissionResult::FailedNetworkError
}

Expected Behavior

Both code paths should return SubmissionResult::Indeterminate when a timeout is detected. The string "timeout" in classify_by_message should map to Indeterminate, consistent with the typed logic.

Why This Matters

If a timeout occurs, we cannot guarantee the transaction wasn't already processed by the cluster. Labeling it as a FailedNetworkError may trigger unsafe retry logic in downstream consumers that assumes the transaction never reached the leader, potentially causing duplicate submissions.

Proposed Changes

  1. Immediate fix: Update classify_by_message to return Indeterminate when a timeout string is detected, aligning with the typed error logic.

  2. Refactor opportunity: Evaluate whether classify_by_message can be deprecated entirely in favor of expanded typed error handling. The string-matching approach is fragile — it breaks if Solana node error message formats change — while the typed path (lines 106-158) already covers 95%+ of real-world cases. Currently classify_by_message is only reachable via ClientErrorKind::Custom(_) (line 156).

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No fields configured for Task.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions