fix(airplay): carry the HTTP status code in the error instead of re-parsing the message

[nikhilshastry2003]

The follow-up #44 named as worth doing on its own: "the status code is not carried through AirPlayError::Negotiation, so there is nothing better to match on today."

post_stream turned a failed status line into the text of a Negotiation error, and run_session parsed the code back out of that text to decide whether to fall back into HAP pairing. #44 made that parse read the first line only, which fixed the header-digit false positives, but the design stayed: the message format was a contract between two functions that nothing enforced. Reword the error and the retry silently stops.

What changes

• AirPlayError::HttpStatus { request, code, status_line } is what a non-success response becomes. The code travels as a number; the status line is kept verbatim for the human reading it. The message is unchanged: POST /stream failed: HTTP/1.1 404 Not Found.
• should_retry_with_pairing(&AirPlayError) replaces wants_authentication(&str) + status_code(&str) in session.rs. It matches on code: 401 | 403 | 404 | 470 | 501 — the same five statuses fix(airplay): stop refusing receivers by model string, and widen the auth retry #44 established, with its receiver table kept in the doc comment. A Negotiation error whose text happens to quote a status line is no longer a retry, because there is no text for a number to be mis-read out of.
• One status-line parser. The crate had three: http_session::status_is_success, session::status_code, and the inline one in hap_pairing::check_http_status. http_session::status_code is now the only one; status_is_success, the new http_failure and check_http_status all go through it. Behaviour matches the two production parsers (second whitespace token of the first line).
• The encrypted POST /stream check in negotiate_with_auth used status_line.contains("200") — the substring pattern fix(airplay): stop refusing receivers by model string, and widen the auth retry #44 removed one layer up. It now goes through status_is_success and produces the same typed error.

Tests

The auth-fallback tests keep every case from #44 — the real receiver statuses (404 / 470 / 401), the original two (501 / 403), and each retryable-code-inside-a-header regression — but build the real error through http_failure, so they exercise the path production takes rather than a hand-made string.

New: a typed error carries the right code and status line; its Display reads as before; an unreadable status line stays an untyped Negotiation; a Negotiation error quoting HTTP/1.1 404 Not Found does not retry; status_code reads only the first line.

Verified

• cargo fmt --all -- --check clean
• cargo clippy -p openplay-airplay --all-targets --all-features -- -D warnings clean
• cargo test -p openplay-airplay — 71 + 27 passed (was 66 + 27)

Only openplay-airplay is touched, so unlike #47 everything here was compiled and run locally; nothing is left to CI alone.

[Developer1010x]

Reviewed. The design is right and I could not find a behavioural defect — but one of the tests does not guard what it appears to, and it is the one protecting this PR's central claim.

The change is sound

Moving the retry decision from wants_authentication(&str) to should_retry_with_pairing(&AirPlayError) removes a contract that nothing enforced, and collapsing three status-line parsers into one is the right cleanup. Reusing the crate's own parser in check_http_status also preserves its behaviour on an empty response and on a first line that is not a status line.

The thing worth checking on a change like this is whether narrowing the guard to HttpStatus silently drops a retry path, since Negotiation used to qualify on text alone. It does not. Every Negotiation site on master that could reach the guard:

• http_session::post_stream — the one converted to HttpStatus. Still retries.
• session.rs lines 336, 341 and 356 — all inside negotiate_with_auth, i.e. past the decision point, so they could never have reached the guard.
• get_info never checks the status at all; it goes straight to parse_info_response, which returns Plist or Ok, never Negotiation.

So the retry set is unchanged in practice, not just in the codes listed.

Verified locally on the whole workspace rather than just openplay-airplay: fmt --check clean, clippy --all-targets --all-features -D warnings clean, cargo test --all 327 passing / 0 failures.

The finding: does_not_retry_on_untyped_or_unrelated_failures is vacuous

The Negotiation case is the assertion that carries the PR's argument — "a Negotiation error whose text happens to quote a status line is no longer a retry":

assert!(!should_retry_with_pairing(&AirPlayError::Negotiation(
    "POST /stream failed: HTTP/1.1 404 Not Found".to_string()
)));

status_code reads split_whitespace().nth(1) off the first line. On that string the second token is /stream, which does not parse — so the input is unreadable whatever the guard does, and the assertion holds for a reason unrelated to the typed error.

Demonstrated by mutation. Regressing the guard to trust message text again:

fn should_retry_with_pairing(err: &AirPlayError) -> bool {
    if let AirPlayError::Negotiation(msg) = err {
        if let Some(c) = crate::http_session::status_code(msg) {
            return matches!(c, 401 | 403 | 404 | 470 | 501);
        }
    }
    matches!(err, AirPlayError::HttpStatus { code: 401 | 403 | 404 | 470 | 501, .. })
}

cargo test -p openplay-airplay still passes, 71/71. The suite does not notice the regression this PR exists to prevent.

Changing only the test input to a bare status line fixes that:

assert!(!should_retry_with_pairing(&AirPlayError::Negotiation(
    "HTTP/1.1 404 Not Found".to_string()
)));

With the same mutation applied, that fails immediately:

assertion failed: !should_retry_with_pairing(&AirPlayError::Negotiation("HTTP/1.1 404 Not Found".to_string()))

"POST /stream failed: ..." is the message an old-style error carried, so choosing it reads as deliberate — but the parser it now faces cannot read a prefixed line, which is exactly why it no longer tests anything. A bare status line is the input that can be mis-read, so it is the one the assertion needs.

This is the same shape as the note in #44 about single-line test strings: the header-matching bug survived its tests because they passed a shape production never produces. Here the test passes a shape the parser cannot parse.

Not a blocker — the shipped behaviour is correct. It is the regression guard that is weak, and it is a one-line fix.

Unrelated, but confirms the gate in #44

Ran pair_probe against a real Mac (Mac16,12, AirPlay version 960.13.1) while reviewing this. It advertises features: 0x38174FDE4A7FCFD5 with the transient-pairing bit set, so #44's decision to gate on feature bit 48 rather than the model string holds up against current hardware. Pair-setup itself returned 403 from the "Current User" AirPlay Receiver setting, which the error text diagnoses correctly — so issue #27's SRP question is still open rather than answered either way.