Skip to content

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

Open
nikhilshastry2003 wants to merge 1 commit into
masterfrom
fix/airplay-typed-http-status
Open

fix(airplay): carry the HTTP status code in the error instead of re-parsing the message#49
nikhilshastry2003 wants to merge 1 commit into
masterfrom
fix/airplay-typed-http-status

Conversation

@nikhilshastry2003

Copy link
Copy Markdown
Collaborator

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.

…arsing the message

`post_stream` turned a failed status line into the text of an
`AirPlayError::Negotiation`, and `run_session` then 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. Change the wording of the error and the
retry silently stops.

Now the code travels as a number. `AirPlayError::HttpStatus { request, code,
status_line }` is what a non-success response becomes, and the retry
decision (`should_retry_with_pairing`) matches on `code` with the same five
statuses as before. A `Negotiation` error whose text happens to quote a
status line is no longer a retry — there is no text to mis-read a number out
of. The message a user sees is unchanged: `POST /stream failed: HTTP/1.1 404
Not Found`.

The crate also had three status-line parsers — `status_is_success`,
`status_code` in session.rs and the inline one in
`hap_pairing::check_http_status`. There is one now, `http_session::status_code`,
and the other two call it. The encrypted `POST /stream` check in
`negotiate_with_auth` used `status_line.contains("200")`, the substring
pattern #44 removed one layer up; it goes through `status_is_success` too.

Tests: the auth-fallback tests keep every case from #44 (the real receiver
statuses, the original two, the retryable-code-inside-a-header regressions)
but now build the real error through `http_failure`, so they exercise the
path production takes. New: a typed error carries the right code and
status line, its message reads as before, an unreadable status line stays
untyped, and a `Negotiation` error quoting `HTTP/1.1 404` does not retry.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant