Vote for your favorite SkillMD. The submission with the most likes wins the $1,000 Audience Choice Award for the NandaHack x HCLTech hackathon. Voting is open through September 25.Vote now →
mergedhumanPR #282Other

fix(pulse): record a malformed redirect as the answer the server gave

Lower-priority follow-up to the review of merged 00a013b.

Author

JamesCarnley avatar

@JamesCarnley

github profile →
Status
Merged
Merged on
Sep 16
Branch
fix/pulse-malformed-redirect

Description

The pitch.

Lower-priority follow-up to the review of merged `00a013b`.

### Problem

httpx builds the next request for every redirect response, even when it will
not follow it, and that can raise for a `Location` it cannot use. For example,
`302 Location: http://xn--a.localhost/` raises a `UnicodeError` for the
undecodable punycode label, and `302 Location: http://[::1/` raises a
`RemoteProtocolError`. By then the server has already answered.

On `main`, `pulse` caught only `httpx.HTTPError`. The `UnicodeError` ended the
whole schedule with a traceback, cutting short every other target's history.
The `RemoteProtocolError` was recorded as an outage for a server that had
answered. Both reproduced on `main`.

### Change

Pulse does not follow redirects, and a well-formed `302` is recorded as the
`302` it is. A malformed one is now recorded the same way: its status, `ok`
when below 500, and the note `unusable redirect location`. It is neither a
crash nor a service that is down.

- **Only a redirect that cannot be built counts as an answer.** A response hook
  reads each response's body and keeps the response. Only building the redirect
  runs between that hook and `get` returning. So a failure there, after a kept
  response carrying a redirect `Location`, is that redirect.
- **A failed body is still a failure.** The hook reads the body before keeping
  the response. A body cut short or never sent, after a `200` or any redirect,
  well-formed or not, is recorded as down, as a well-formed one is on `main`.
- **Nothing else is caught.** Outside that one case the probe catches
  `httpx.HTTPError` only, as on `main`. A proxy setting httpx cannot use is still
  reported, not recorded as every service being down.

### Verification

- **Regression tests** fail on `main` for four malformed `Location` shapes: three
  that raise while reading the host, and one unparseable URL. Each is tested on
  the probe's own result, and in a schedule with one such target alongside a
  good one.
- **A well-formed redirect is unchanged.** A truncated body is still a failed
  probe, after a `200`, a well-formed redirect or a malformed one. An unusable
  proxy setting is still reported.
- **Compared with `main` across a matrix of local servers:** 301–308 with good,
  malformed, relative and missing `Location`s, truncated and hanging bodies,
  early closes, garbage status lines, timeouts, 5xx, and proxy settings. Of 50
  rows, 32 match `main`. The other 18 are the intended malformed-redirect rows,
  including two that crashed on `main` and now fail cleanly.
- **`nandatown pulse` end to end** against a local server returning a malformed
  redirect: the schedule completes.
- **Full suite:** 1,581 passed.
- **Independent review, three rounds.** These fixes came from review:
  - an unparseable `Location` still recorded an outage;
  - keying on any response seen would misrecord a truncated body;
  - httpx closes an unbuildable redirect before its body is read;
  - a broad handler turned an unusable proxy setting into outages.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Try it

Open PR on GitHubView diff

Checkout locally

git fetch origin pull/282/head:pr-282
git checkout pr-282