Plans (docs/plans/): - 2026-07-01-23-47-py-hxprobe-httpx.md — initial httpx probe design - 2026-07-02-09-32 through 14-05 — standalone project, toolchain, usage doc + Makefile, file input (-f), simplification pass, run-summary footer Summaries (docs/summaries/): one per completed feature, recording what was actually built, deviations from the plan, and verification steps Explanations (docs/explanations/): two deep-dives written during review — hxprobe concurrency model and worst-exit-code + render-loop analysis Usage (docs/usage/hxprobe.md): overview with pointer to hxprobe/USAGE.md for the full runnable reference Walkthrough (docs/py-latprobe-walkthrough.md): narrative tour of the latprobe Python package for interview / code-review context CHANGELOG.md: entries for all hxprobe features (toolchain, usage doc, file input, simplification, run-summary footer) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
145 lines
6.0 KiB
Markdown
145 lines
6.0 KiB
Markdown
# What does the per-URL worst-exit-code / render loop do, and does it make sense?
|
|
|
|
## Question
|
|
|
|
Selection in `hxprobe/hxprobe/cli.py:454-472`:
|
|
```python
|
|
for i, (url, (succeeded, failed)) in enumerate(zip(urls, all_results)):
|
|
for r in failed:
|
|
c = _phase_code(r.fail_phase)
|
|
if c > worst:
|
|
worst = c
|
|
if ns.fail:
|
|
for r in succeeded:
|
|
if r.status_code >= 400:
|
|
worst = max(worst, EXIT_HTTP)
|
|
|
|
last_detail = succeeded[-1].detail if succeeded else (failed[-1].detail if failed else None)
|
|
|
|
if ns.json_out:
|
|
json_items.append(_build_json_entry(url, succeeded, failed, last_detail))
|
|
continue
|
|
|
|
if i > 0:
|
|
stdout.write("\n")
|
|
_print_url(url, succeeded, failed, count, ns.fail, stdout)
|
|
```
|
|
> explain following block and if it makes sense, other related stuff, and save it in explanations
|
|
|
|
## What it does
|
|
|
|
This is the single pass over per-URL results that runs after the thread pool
|
|
(`cli.py:448-449`, see [2026-07-02-13-10-hxprobe-concurrency.md](2026-07-02-13-10-hxprobe-concurrency.md))
|
|
finishes. It does two jobs in one loop: compute the process's final exit code,
|
|
and render output (text or accumulate JSON) — one URL at a time, in input
|
|
order (guaranteed by `zip(urls, all_results)` since `ex.map` preserves order).
|
|
|
|
**1. Network-failure exit code (`cli.py:455-458`)**
|
|
```python
|
|
for r in failed:
|
|
c = _phase_code(r.fail_phase)
|
|
if c > worst:
|
|
worst = c
|
|
```
|
|
Every failed sample (across every URL, since `worst` is declared once before
|
|
the loop) is mapped to an exit code via `_phase_code` / `_PHASE_EXIT`
|
|
(`cli.py:24-32`):
|
|
```python
|
|
_PHASE_EXIT: dict[str, int] = {
|
|
"dns": EXIT_DNS, # 2
|
|
"timeout": EXIT_TIMEOUT, # 4
|
|
"tls": EXIT_TLS, # 5
|
|
}
|
|
def _phase_code(fail_phase: str) -> int:
|
|
return _PHASE_EXIT.get(fail_phase, EXIT_CONNECT) # 3, the fallback
|
|
```
|
|
Any `fail_phase` not in the table — `"connect"`, `"transfer"`, `"request"` —
|
|
falls back to `EXIT_CONNECT` (3). `worst` tracks the running max across all
|
|
URLs/samples, so the process exit code always reflects the single worst
|
|
failure class seen, per the exit-code table (0 ok … 6 http via `--fail`).
|
|
|
|
**2. `--fail` (HTTP status ≥ 400) exit code (`cli.py:459-462`)**
|
|
```python
|
|
if ns.fail:
|
|
for r in succeeded:
|
|
if r.status_code >= 400:
|
|
worst = max(worst, EXIT_HTTP)
|
|
```
|
|
Only runs when `--fail` is passed. Note this scans `succeeded` — a 404 is not
|
|
a network failure, so those `Result`s land in `succeeded` with a populated
|
|
`status_code`; `--fail` is what turns "successfully got a bad status" into a
|
|
non-zero exit, curl-style.
|
|
|
|
**3. Verbose-detail selection (`cli.py:464`)**
|
|
```python
|
|
last_detail = succeeded[-1].detail if succeeded else (failed[-1].detail if failed else None)
|
|
```
|
|
Prefers the last successful sample's `detail` (freshest full picture: IP,
|
|
protocol, TLS, headers); falls back to the last *failed* sample's `detail` if
|
|
nothing succeeded (e.g. resolved IP is still known even on a connection
|
|
refusal); `None` if there's nothing to show. Reasonable design — surfaces
|
|
partial diagnostic info even on total failure.
|
|
|
|
**4. JSON accumulation vs. text rendering (`cli.py:466-472`)**
|
|
```python
|
|
if ns.json_out:
|
|
json_items.append(_build_json_entry(url, succeeded, failed, last_detail))
|
|
continue
|
|
|
|
if i > 0:
|
|
stdout.write("\n")
|
|
_print_url(url, succeeded, failed, count, ns.fail, stdout)
|
|
```
|
|
JSON mode builds up `json_items` (dumped once after the loop) and skips
|
|
straight to the next URL via `continue`. Text mode writes a blank-line
|
|
separator before every URL block except the first (`i > 0`), then delegates
|
|
actual formatting to `_print_url`, which picks one of four branches
|
|
(single / aggregate / all-failed / mixed) based on `n_ok`/`n_fail`/`total_count`.
|
|
|
|
## Does it make sense?
|
|
|
|
**Yes, structurally.** Combining exit-code accumulation and rendering into one
|
|
O(n) pass is reasonable for a CLI at this scale — no need to split into two
|
|
loops. The `last_detail` fallback logic is a genuinely good touch. The one
|
|
stylistic wrinkle — the DNS/connect/timeout/tls loop uses
|
|
`if c > worst: worst = c` while the `--fail` branch uses `worst = max(worst,
|
|
EXIT_HTTP)` for the same "keep the running max" purpose — is harmless
|
|
inconsistency, not a bug.
|
|
|
|
**Update (2026-07-02):** the "does a single worst-code exit even make sense
|
|
across multiple URLs with different errors" question came back as a follow-up
|
|
and led to a real feature — see
|
|
[2026-07-02-14-05-hxprobe-run-summary-footer.md](2026-07-02-14-05-hxprobe-run-summary-footer.md).
|
|
Short version: the scalar exit code is kept (it's a documented cross-
|
|
implementation contract with `latprobe`/Go), but multi-URL runs now get an
|
|
end-of-run summary footer tallying every URL's outcome, so the "worst code"
|
|
is no longer the only visibility into what happened. That change also
|
|
unified the two idioms noted above into one `max(...)` call.
|
|
|
|
**One real gap, found while checking this: `-n`/`--count` is unvalidated.**
|
|
`cli.py:349-355` declares `--count` as `type=int, default=1` with no minimum.
|
|
`_run_samples` (`cli.py:64-69`) does `for _ in range(count): ...`, so
|
|
`--count 0` (or any negative value) makes the loop body never execute, and
|
|
both `succeeded` and `failed` come back empty for that URL. Confirmed live:
|
|
|
|
```
|
|
$ hxprobe --count 0 https://example.com
|
|
(0, 0 samples)
|
|
exit=0
|
|
|
|
$ hxprobe --count -2 https://example.com
|
|
(0, 0 samples)
|
|
exit=0
|
|
```
|
|
|
|
The URL itself is missing from the header, `status` reads `0`, and the exit
|
|
code is `0` (success) — because `_print_url` falls into the `elif n_fail == 0`
|
|
aggregate branch with `summarize([])`, which returns a bare
|
|
`Aggregate()` (all defaults, `url=""`) rather than anything referencing the
|
|
actual `url` variable. This is silent garbage output instead of a clear
|
|
usage error, and it's inconsistent with how the rest of `run()` already
|
|
validates arguments (e.g. the `parser.error(...)` calls for the
|
|
`urls`/`--file` mutual-exclusion checks at `cli.py:407-410`). Worth a
|
|
`parser.error("count must be >= 1")`-style guard if this is ever picked up —
|
|
not fixed here since it wasn't asked for, just flagged as a finding.
|