Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
6e9a36903e
|
||
|
|
f6fa43a8b4
|
||
|
|
f4562cac26
|
||
|
|
250bdca7e7
|
||
|
|
63f4f96a48
|
||
|
|
52158198cf
|
||
|
|
bf79a22584
|
@@ -4,6 +4,18 @@ All notable changes to nxdns are recorded here. The format follows [Keep a Chang
|
||||
|
||||
Sections are written by hand. Nothing here is generated from commit messages: the point of the file is to say what changed for an operator, which a commit subject rarely does.
|
||||
|
||||
## [0.0.20] - 2026-09-12
|
||||
|
||||
### Fixed
|
||||
|
||||
- **A blocklist source no longer fails on a stale pooled connection.** The fetcher reused a keep-alive connection from an earlier pass that the server had closed, and the standard client never retries one; every later download from that host failed with `HttpConnectionClosing` at 0 ms. Downloads now send `connection: close` and never enter the pool.
|
||||
|
||||
## [0.0.19] - 2026-09-12
|
||||
|
||||
### Changed
|
||||
|
||||
- **A failed blocklist download says why.** The warning line and the Diagnostics event detail now carry the phase that failed, the concrete cause behind the classification (a reset connection, a truncated chunk, a TLS handshake fault), the HTTP status if a head arrived, the bytes received, and the elapsed time. The separate `http status` warning is folded into that line. Same one line per failed source per pass as before.
|
||||
|
||||
## [0.0.18] - 2026-09-09
|
||||
|
||||
### Fixed
|
||||
|
||||
+1
-1
@@ -1,6 +1,6 @@
|
||||
.{
|
||||
.name = .nxdns,
|
||||
.version = "0.0.18",
|
||||
.version = "0.0.20",
|
||||
.minimum_zig_version = "0.16.0",
|
||||
.paths = .{""},
|
||||
.fingerprint = 0x3307b311dded1d91,
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# Install nxdns with Nix
|
||||
|
||||
Adds nxdns to a NixOS machine as a flake input pinned to a release tag. At the end `pkgs`-style references to `inputs.nxdns.packages.${system}.default` resolve to the published release binary, and Renovate opens a pull request when a new tag appears.
|
||||
Adds nxdns to a NixOS machine as a flake input pinned to a release tag. At the end `pkgs`-style references to `inputs.nxdns.packages.${system}.default` resolve to the published release binary, and a Renovate custom manager opens a pull request when a new tag appears.
|
||||
|
||||
nxdns publishes its own `flake.nix`. Its packages do not build nxdns from source: each one fetches the release tarball for the target and pins its SHA-256 hash, so a changed byte fails the build. The two supported systems are `aarch64-linux` and `x86_64-linux`, both static musl builds that need nothing on the host.
|
||||
|
||||
@@ -57,19 +57,29 @@ The derivation installs `bin/nxdns`, plus `LICENSE` and `THIRD-PARTY-NOTICES` un
|
||||
|
||||
## 4. Let Renovate bump the tag
|
||||
|
||||
Enable the Nix manager in the consuming repository's `renovate.json`:
|
||||
Renovate's built-in `nix` manager does not do this. It only advances `flake.lock` along the ref an input already tracks, and a `refs/tags/vX.Y.Z` ref never moves, so it reports no releases for a tag-pinned input. Verified on 2026-09-09 with Renovate 42.99.0 against a Gitea host: all flake inputs, GitHub-hosted ones included, came back with an empty release list.
|
||||
|
||||
What works is a regex custom manager that treats the tag in the input URL as a version string, with the `gitea-tags` datasource:
|
||||
|
||||
```json
|
||||
{
|
||||
"$schema": "https://docs.renovatebot.com/renovate-schema.json",
|
||||
"extends": ["config:recommended"],
|
||||
"nix": {
|
||||
"enabled": true
|
||||
}
|
||||
"customManagers": [
|
||||
{
|
||||
"customType": "regex",
|
||||
"managerFilePatterns": ["/^flake\\.nix$/"],
|
||||
"matchStrings": ["git\\+https://git\\.mial\\.net/mokhtar/nxdns\\.git\\?ref=refs/tags/(?<currentValue>v\\d+\\.\\d+\\.\\d+)"],
|
||||
"depNameTemplate": "mokhtar/nxdns",
|
||||
"datasourceTemplate": "gitea-tags",
|
||||
"registryUrlTemplate": "https://git.mial.net",
|
||||
"versioningTemplate": "semver"
|
||||
}
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
Renovate reads the flake inputs and the lock file, sees the `refs/tags/v0.0.16` ref, and opens a pull request when a newer tag exists. It proposes only tags that already exist, so it never pins a release that has not been cut.
|
||||
Verified on 2026-09-09: with this manager, Renovate opened the pull request for v0.0.18 on the day of the tag. That pull request rewrites the tag in `flake.nix` only. `flake.lock` still records the old revision, so the same pull request must also refresh the lock, either with a `postUpgradeTasks` command (`nix flake update nxdns`, which needs the self-hosted `allowedCommands` setting) or with a CI job on the Renovate branch that commits the lock. The datasource proposes only tags that already exist, so it never pins a release that has not been cut.
|
||||
|
||||
Between the tag push and the asset upload there is a window in which the tag exists and the release tarballs do not. A pin written by hand during that window evaluates, then fails at build time with a 404 from the release download URL. Wait for the release to be published, or re-run the build once it is. Renovate's own pull requests are not affected by the window in practice, because it runs on a schedule rather than on the tag push.
|
||||
|
||||
|
||||
@@ -8,10 +8,10 @@
|
||||
let
|
||||
# `zig build cut` rewrites the lines between the delimiters by text replacement.
|
||||
# BEGIN GENERATED BY zig build cut
|
||||
version = "0.0.18";
|
||||
version = "0.0.20";
|
||||
hashes = {
|
||||
"aarch64-linux" = "sha256-7TmuJr6TkgRdPkYvNiEOFVfkzkp+djjWSml+vAUee78=";
|
||||
"x86_64-linux" = "sha256-uBWjl8FvP4VgPisJgydNazyPu40jjQa7+i64+M/B50A=";
|
||||
"aarch64-linux" = "sha256-2p86FImDgi4SDDzmV6w0qkrkp9dtDRyi/ZMOZ8WAEm4=";
|
||||
"x86_64-linux" = "sha256-rnTpjo7J9uY+pINxJ4ZgwbqU7E2o7N0gr9z7VRhQiZ0=";
|
||||
};
|
||||
# END GENERATED BY zig build cut
|
||||
|
||||
|
||||
@@ -81,6 +81,8 @@ Deviation (Session D): the pin stage is tested in pieces rather than on a fixtur
|
||||
|
||||
Signing the flake outputs, a Hydra or cachix binary cache, Darwin packages, and a NixOS module. Renovate tag discovery on Gitea is a consumer-side acceptance test run once on `rpi.mial.net` after the first tagged release with a flake.
|
||||
|
||||
Result of that test (2026-09-09, v0.0.18, Renovate 42.99.0): no pull request. Renovate's `nix` manager advances `flake.lock` along the ref an input already tracks and never moves a `refs/tags/vX.Y.Z` ref; it returned an empty release list for every flake input, GitHub-hosted ones included. The flake itself is correct and v0.0.17 deployed through it. Tag bumps need a regex custom manager with the `gitea-tags` datasource plus a lock refresh in the same pull request, which the consumer owns; `docs/how-to/install-with-nix.md` section 4 now says so instead of claiming the `nix` manager does it. With that manager in place, Renovate opened rpi.mial.net PR 5 the same day, bumping the input to v0.0.18 and refreshing `flake.lock`: the acceptance test passes.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- `zig build test` green, including the two-path reproducibility test.
|
||||
|
||||
@@ -1377,3 +1377,26 @@ Deviations from the text above, recorded after the first six sessions verified.
|
||||
**S9 integration + fuzz.** Case 3 splits into the two real failure modes: a checksum-mismatched file marks the source `.load_failed` and the reload succeeds without it (generation advances); only a checksum-clean but malformed body fails `reload` and leaves the previous snapshot serving. Case 7 proves the cap from the response head: an explicit `content-length` of 100 MiB returns `error.BodyTooLarge` before any body streams. Case 8 back-dates `last_updated` through `updateSourceStats` and asserts inode and mtime of the compiled file are unchanged. Case 15 asserts `budget/2 <= elapsed < 2*budget` (the POSIX backend wakes ~0.8 ms early on a 200 ms deadline). The fuzz corpus is inline — `tests/fuzz/corpus.zig` imports the `dns` module, which the blocklist-fuzz module does not have. Fuzz targets assert properties, not only absence of crashes: `Line.text` windows the input (pointer containment), `covers_apex` only on `.wildcard`, `detectFormat`'s answer survives `parseLine` over the same bytes, and `matches` is exercised on rejected patterns.
|
||||
|
||||
**Final wiring.** `build.zig` gained the `blocklist-fuzz` artifact (module import `parsers` → `src/filter/parsers.zig`, LLVM backend under `-Dfuzz`), hung off `test_step` beside the dns fuzz artifact. Evaluation: `zig build test`, `zig build test -Dintegration` and `zig build cross` all exit 0; both cross executables are statically linked.
|
||||
|
||||
## Addendum (2026-09-12): a download failure names its cause
|
||||
|
||||
Observed on the Pi (nxdns 0.0.17): four of eight sources fail on every refresh pass since 2026-09-11, the same four each time, and the log line for each is `download failed: TlsFailed` one pass and `download failed: ReceiveFailed` the next. The same binary loads all eight from another machine. The line cannot tell a TLS alert from a reset connection from a truncated chunk, because `fetcher.mapError` collapses the cause into the six-member taxonomy and `reportDownloadFailure` logs only the taxonomy name. The Diagnostics event carries the same text. A reader of either has nothing to act on. `upstream/doh_client.zig` already unwraps the concrete cause that `std.http.Client` stashes on the connection (`sendCause`, `headCause`, `bodyCause`, `readCause`); the fetcher does not.
|
||||
|
||||
Change:
|
||||
|
||||
- `fetcher.zig`: `Fetcher` gains `last_failure: ?Failure`, cleared at the top of `fetch` beside `last_status`, set on every error return. `Failure` is `{ phase: Phase, cause: anyerror, status: ?std.http.Status, bytes_read: u64 }`. `Phase` gains `receive_body`, distinct from the head. `cause` is the unwrapped concrete error: the three unwrap helpers move from `doh_client.zig` into `upstream/transport.zig` (or a sibling file both import; the coder picks the smaller diff), keep their tests, and both clients call them. The taxonomy `Error` and `mapError` are unchanged; a caller that ignores `last_failure` sees exactly what it saw before. `bytes_read` counts body bytes delivered to the writer before the failure, so a body that dies at byte 0 and one that dies at 5 MB read differently.
|
||||
- `manager.zig`: `fetchWithin` measures elapsed milliseconds on the awake clock. `reportDownloadFailure` takes the failure and elapsed time and logs one line: `blocklist <label>: download failed: <taxonomy> (<phase>, cause <CauseName>, http <status or none>, <bytes> bytes, <ms> ms)`. On the `Timeout` outcome of the expiry race the losing fetch is cancelled before the line is built, and it records itself on the way out, so the line names the phase it was in, `cause Canceled`, the status if a head arrived, and the bytes delivered so far: `Timeout (receive_body, cause Canceled, http 200, 5242880 bytes, 300000 ms)`. `download` clears `last_failure` and `last_status` before anything else runs, so a null record means no fetch reached the network for this source (a local file failure, or no concurrency to spawn the race); that prints as `<taxonomy> (no fetch, http none, - bytes, <ms> ms)`. Elapsed is captured when the race resolves, before the loser is cancelled, so a timeout's milliseconds are the time the transfer was given. `SourceStatus.fail` receives the same compact text, so the Diagnostics event detail shows it; `max_error_len` rises from 128 to 192 so the longest cause name and the numbers fit without truncation (check the longest `std.crypto.tls.Client.ReadError` and `std.http.Client` error names; `events.Store.max_detail_len` is 512 and is not touched).
|
||||
- Bound: this changes the content of an existing warning line, not its frequency. One line per failed source per pass, at most `sources × passes` per day; a pass runs once per `blocklist_update.interval_hours` or on a manual update. No per-chunk or per-retry logging is added. Nothing new is written at info or debug level.
|
||||
- Tests: `mapError` tests unchanged; a test that a head failure with a stashed `ConnectionResetByPeer` yields `last_failure.cause == error.ConnectionResetByPeer` and phase `receive` (the doh tests' stub connection pattern); a test that a body failure records `bytes_read` and `receive_body`; a `reportDownloadFailure` format test with the fixed text `blocklist source 3 'ads' 'https://lists.example': download failed: ReceiveFailed (receive_body, cause HttpChunkTruncated, http 200, 1048576 bytes, 812 ms)`; the existing secrecy assertions on the label stay.
|
||||
|
||||
Out of scope: a TLS handshake failure stays `TlsInitializationFailed`, because `std.http.Client` collapses every handshake fault at `Client.zig:1470` before this code sees it; distinguishing an alert from a certificate failure would mean driving `std.crypto.tls.Client` by hand. Retries and a per-source backoff are not added.
|
||||
|
||||
## Addendum (2026-09-12): a blocklist download never reuses a pooled connection
|
||||
|
||||
The line above found the cause the same day nxdns 0.0.19 reached the Pi. A manual refresh at 14:28 failed the same four sources with one record each: `ReceiveFailed (receive, cause HttpConnectionClosing, http none, 0 bytes, 0 ms)`. Under one millisecond of elapsed time, and no response head byte after the request was sent, means the peer had already closed the socket. `std.http.Client` returns `HttpConnectionClosing` when a reused connection hits EOF before the first head byte (`std/http.zig:400`), and `std/http/test.zig:1331` pins that a client never retries a stale pooled connection. The startup pass at 13:03 had fetched `big.oisd.nl` successfully and the client parked that keep-alive connection in `fetch_http`'s pool; the server closed it long before 14:28, and the next request to the same host picked it up. The same shape explains every earlier observation: the first fetch to a host in a process succeeds, a later fetch to that host fails, and the daily pass reuses connections a day old. The `TlsFailed` variant of the older lines is the same dead socket failing inside the TLS layer instead of at the head.
|
||||
|
||||
Change: `fetcher.fetch` requests with `.keep_alive = false`, and `fetch_http` in app.zig is created with `connection_pool.free_size = 0`, so `release` destroys every connection regardless of the request flag (`Client.zig:133`), including the one a failed send leaves in the `.ready` state that `Request.deinit` would otherwise pool, and no connection is ever taken from that pool. The flag alone is not enough: `Client.request` takes a pooled connection before it stores the flag (`Client.zig:1725`, `:1742`). The request then carries `connection: close`, the client marks the connection closing at the head, and `release` destroys it instead of pooling it (`Client.zig:133`, `:1167`). A blocklist download is one bulk transfer per source per day; there is nothing to gain from a pooled connection and a stale one costs a source for a day. `fetch_http` stays a separate client from `dns_http`, so the DoH pool is untouched.
|
||||
|
||||
Tests: an integration test against `HttpFixture` where the server closes the socket after each response: two consecutive fetches to the same URL both succeed. Without the change the second fails with `HttpConnectionClosing`, which is the exact record the Pi produced. A unit test that the request header the fetcher sends carries `connection: close` if the fixture route is the cheaper path.
|
||||
|
||||
Out of scope: a retry on `HttpConnectionClosing` for the DoH client, which keeps pooling by design and has its own health logic.
|
||||
|
||||
+13
-1
@@ -482,7 +482,19 @@ fn serve(r: cli.Runner, args: cli.RunArgs) !u8 {
|
||||
// Two HTTP clients on purpose. A blocklist download streams tens of
|
||||
// megabytes and holds its connection for the whole of it; DoH queries must
|
||||
// not queue behind that, and the two have nothing to share but a type.
|
||||
var fetch_http: std.http.Client = .{ .allocator = gpa, .io = io };
|
||||
// `free_size = 0` makes `release` destroy every connection and the pool
|
||||
// hand none back (`Client.zig:133`). The fetcher's `.keep_alive = false` is
|
||||
// what sends `connection: close`, but it cannot carry this alone: `request`
|
||||
// takes a pooled connection before it stores the flag (`Client.zig:1725`,
|
||||
// `:1742`), and a failed send leaves a `.ready` connection that
|
||||
// `Request.deinit` would pool (`Client.zig:890`). A pass runs once a day,
|
||||
// so any kept connection is closed at the peer by the next one, and the
|
||||
// client never retries a stale one — it fails the source for the day.
|
||||
var fetch_http: std.http.Client = .{
|
||||
.allocator = gpa,
|
||||
.io = io,
|
||||
.connection_pool = .{ .free_size = 0 },
|
||||
};
|
||||
defer fetch_http.deinit();
|
||||
var dns_http: std.http.Client = .{ .allocator = gpa, .io = io };
|
||||
defer dns_http.deinit();
|
||||
|
||||
+312
-33
@@ -1,16 +1,21 @@
|
||||
//! Blocklist download over HTTP/1.1.
|
||||
//!
|
||||
//! One `Fetcher` wraps a caller-owned `std.http.Client`, which owns the
|
||||
//! connection pool and the CA bundle, exactly as `upstream/doh_client.zig`
|
||||
//! does. This file knows nothing about parsing, files or the database: it GETs
|
||||
//! a URL and streams the bytes into a writer the caller supplies.
|
||||
//! One `Fetcher` wraps a caller-owned `std.http.Client`, which owns the CA
|
||||
//! bundle, as `upstream/doh_client.zig` does. Nothing here is pooled: every
|
||||
//! request asks for `connection: close`, and the caller is expected to give
|
||||
//! this file a client whose pool holds nothing (`free_size = 0`, set at the
|
||||
//! one construction site in `app.zig`). This file knows nothing about parsing,
|
||||
//! files or the database: it GETs a URL and streams the bytes into a writer
|
||||
//! the caller supplies.
|
||||
//!
|
||||
//! The body is never held whole. A blocklist can reach `max_body_bytes`, and
|
||||
//! the caller writes into a temporary file anyway, so nothing here allocates.
|
||||
//!
|
||||
//! There is no timeout parameter and no sleep. `std.http.Client` has no
|
||||
//! per-request deadline, so the caller runs `fetch` under `io.concurrent` and
|
||||
//! cancels the future; this file only propagates `error.Canceled`.
|
||||
//! cancels the future. A cancel during the body copy surfaces as
|
||||
//! `error.ReceiveFailed` with `Canceled` in `last_failure`; elsewhere it
|
||||
//! propagates as `error.Canceled`.
|
||||
|
||||
const std = @import("std");
|
||||
const transport = @import("../upstream/transport.zig");
|
||||
@@ -43,6 +48,30 @@ pub const Result = struct {
|
||||
status: std.http.Status,
|
||||
};
|
||||
|
||||
/// The concrete fault behind an `Error`. `cause` is unwrapped from whatever
|
||||
/// `std.http.Client` stashed on the connection or the response, so it names a
|
||||
/// record-layer TLS fault or an HTTP framing fault rather than the collapsed
|
||||
/// `error.ReadFailed`.
|
||||
pub const Failure = struct {
|
||||
phase: Phase,
|
||||
cause: anyerror,
|
||||
/// The response status if a head arrived before the failure.
|
||||
status: ?std.http.Status,
|
||||
/// Body bytes delivered to the caller's writer before the failure, and a
|
||||
/// lower bound rather than an exact count: at least this many reached `w`.
|
||||
/// A single `Reader.stream` call can hand bytes to `w` and then fail, and
|
||||
/// that partial delivery is not counted. Zero on every phase before
|
||||
/// `receive_body`.
|
||||
bytes_read: u64,
|
||||
};
|
||||
|
||||
/// Which call failed. The phase is what decides the classification, and only
|
||||
/// the call site knows it — guessing it from an error name would be wrong the
|
||||
/// first time two phases shared an error. `receive_body` is distinct from
|
||||
/// `receive`: the head arrived, so the fault is in the transfer, not the
|
||||
/// response.
|
||||
pub const Phase = enum { connect, send, receive, receive_body };
|
||||
|
||||
pub const Fetcher = struct {
|
||||
/// Caller-owned; shared across sources, pools connections.
|
||||
http: *std.http.Client,
|
||||
@@ -55,6 +84,12 @@ pub const Fetcher = struct {
|
||||
/// one. `error.HttpStatus` carries no `Result`, and the operator's message
|
||||
/// needs the number, so it is readable here after a failed `fetch`.
|
||||
last_status: ?std.http.Status = null,
|
||||
/// What the most recent `fetch` failed on, or null if it succeeded or has
|
||||
/// not run. The taxonomy `Error` a caller receives names six outcomes; this
|
||||
/// names the one concrete fault behind the outcome, so an operator reading
|
||||
/// the log can tell a TLS alert from a reset connection from a truncated
|
||||
/// chunk.
|
||||
last_failure: ?Failure = null,
|
||||
|
||||
/// GETs `url` and streams the body into `w`.
|
||||
pub fn fetch(
|
||||
@@ -71,37 +106,110 @@ pub const Fetcher = struct {
|
||||
std.debug.assert(self.transfer_buf.len >= min_transfer_buf);
|
||||
std.debug.assert(self.redirect_buf.len >= redirect_buffer_len);
|
||||
|
||||
const uri = try parseUrl(url);
|
||||
|
||||
self.last_status = null;
|
||||
self.last_failure = null;
|
||||
|
||||
const uri = parseUrl(url) catch |err| return self.record(err, .connect, err, null, 0);
|
||||
|
||||
var req = self.http.request(.GET, uri, .{
|
||||
.keep_alive = true,
|
||||
// Sends `connection: close`, so the peer ends the connection and
|
||||
// `release` destroys it rather than pooling it (`Client.zig:133`,
|
||||
// `:1167`). A pass runs once a day: a kept connection is one the
|
||||
// peer has already closed by the next pass, and `std.http.Client`
|
||||
// never retries a reused connection that reaches EOF before the
|
||||
// first head byte — it returns `error.HttpConnectionClosing` and
|
||||
// the source fails for the day. This flag cannot carry that alone,
|
||||
// because `request` takes a pooled connection before it reads the
|
||||
// flag; `app.zig` empties the pool itself.
|
||||
.keep_alive = false,
|
||||
.headers = .{
|
||||
// Identity only: a compressed transfer encoding would need
|
||||
// `Response.readerDecompressing`, a decompression buffer and a
|
||||
// second failure surface, for a download that runs once a day.
|
||||
.accept_encoding = .{ .override = "identity" },
|
||||
},
|
||||
}) catch |err| return mapError(err, .connect);
|
||||
}) catch |err| return self.record(mapError(err, .connect), .connect, err, null, 0);
|
||||
defer req.deinit();
|
||||
|
||||
req.sendBodiless() catch |err| return mapError(err, .send);
|
||||
req.sendBodiless() catch |err|
|
||||
return self.record(mapError(err, .send), .send, transport.sendCause(&req, err), null, 0);
|
||||
|
||||
var resp = req.receiveHead(self.redirect_buf) catch |err| return mapError(err, .receive);
|
||||
var resp = req.receiveHead(self.redirect_buf) catch |err|
|
||||
return self.record(mapError(err, .receive), .receive, transport.headCause(&req, err), null, 0);
|
||||
|
||||
self.last_status = resp.head.status;
|
||||
if (resp.head.status != .ok) return error.HttpStatus;
|
||||
if (resp.head.status != .ok) {
|
||||
return self.record(error.HttpStatus, .receive, error.HttpStatus, resp.head.status, 0);
|
||||
}
|
||||
|
||||
// `content-type` is deliberately not checked: blocklists are served as
|
||||
// text/plain, application/octet-stream and text/html alike, and the
|
||||
// compiler's invalid-line counters are the honest signal about content.
|
||||
if (resp.head.content_length) |declared| {
|
||||
if (declared > max_body_bytes) return error.BodyTooLarge;
|
||||
if (declared > max_body_bytes) {
|
||||
return self.record(
|
||||
error.BodyTooLarge,
|
||||
.receive,
|
||||
error.BodyTooLarge,
|
||||
resp.head.status,
|
||||
0,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
const body = resp.reader(self.transfer_buf);
|
||||
return .{ .bytes_read = try pumpBody(body, w, max_body_bytes), .status = resp.head.status };
|
||||
var pump: Pump = .{};
|
||||
const bytes_read = pumpBody(body, w, max_body_bytes, &pump) catch |err| return self.record(
|
||||
err,
|
||||
.receive_body,
|
||||
transport.bodyCause(&resp, pump.cause),
|
||||
resp.head.status,
|
||||
pump.delivered,
|
||||
);
|
||||
return .{ .bytes_read = bytes_read, .status = resp.head.status };
|
||||
}
|
||||
|
||||
/// Stores the failure and hands back `mapped` unchanged.
|
||||
///
|
||||
/// `mapped` is the taxonomy error the call site already decided, never one
|
||||
/// this function derives. `mapError` answers for a raw std error, and the
|
||||
/// call sites that raise a taxonomy member directly — a non-200 head, a
|
||||
/// declared length over the cap — pass that member: feeding either back
|
||||
/// through `mapError` would find no case for it and fall to the phase
|
||||
/// default, turning `error.HttpStatus` into `error.ReceiveFailed`.
|
||||
///
|
||||
/// `cause` never reaches the returned error. The unwrap is for the
|
||||
/// operator's line, and a caller that ignores `last_failure` sees exactly
|
||||
/// what it saw before the unwrap existed.
|
||||
fn record(
|
||||
self: *Fetcher,
|
||||
mapped: Error,
|
||||
phase: Phase,
|
||||
cause: anyerror,
|
||||
status: ?std.http.Status,
|
||||
bytes_read: u64,
|
||||
) Error {
|
||||
self.last_failure = .{
|
||||
.phase = phase,
|
||||
.cause = cause,
|
||||
.status = status,
|
||||
.bytes_read = bytes_read,
|
||||
};
|
||||
return mapped;
|
||||
}
|
||||
};
|
||||
|
||||
/// What `pumpBody` reports alongside its error: the reader error before
|
||||
/// `mapError` collapses it, and the bytes already handed to `w`. `pumpBody`
|
||||
/// knows nothing of `std.http`, so the unwrap of `cause` happens in `fetch`,
|
||||
/// which holds the response.
|
||||
const Pump = struct {
|
||||
cause: anyerror = error.Unexpected,
|
||||
delivered: u64 = 0,
|
||||
|
||||
fn fail(self: *Pump, cause: anyerror, mapped: Error) Error {
|
||||
self.cause = cause;
|
||||
return mapped;
|
||||
}
|
||||
};
|
||||
|
||||
@@ -116,17 +224,19 @@ pub const Fetcher = struct {
|
||||
/// `self.transfer_buf` — makes `@memcpy` copy the buffer onto itself
|
||||
/// (Reader.zig:677) and panics the process on the first read that finds bytes
|
||||
/// already buffered.
|
||||
fn pumpBody(body: *std.Io.Reader, w: *std.Io.Writer, cap: usize) Error!u64 {
|
||||
fn pumpBody(body: *std.Io.Reader, w: *std.Io.Writer, cap: usize, out: *Pump) Error!u64 {
|
||||
var total: usize = 0;
|
||||
while (total < cap) {
|
||||
out.delivered = total;
|
||||
total += body.stream(w, .limited(cap - total)) catch |err| switch (err) {
|
||||
error.EndOfStream => return total,
|
||||
// The caller owns `w` and can read the concrete failure from its
|
||||
// own writer; this taxonomy has no member for a failing sink.
|
||||
error.WriteFailed => return error.Unexpected,
|
||||
else => |e| return mapError(e, .receive),
|
||||
error.WriteFailed => return out.fail(err, error.Unexpected),
|
||||
else => |e| return out.fail(e, mapError(e, .receive_body)),
|
||||
};
|
||||
}
|
||||
out.delivered = total;
|
||||
|
||||
// `cap` written exactly. One more byte separates a body that fits from one
|
||||
// that was cut off, and it must not reach `w`.
|
||||
@@ -134,10 +244,10 @@ fn pumpBody(body: *std.Io.Reader, w: *std.Io.Writer, cap: usize) Error!u64 {
|
||||
var probe: std.Io.Writer.Discarding = .init(&probe_buf);
|
||||
const extra = body.stream(&probe.writer, .limited(1)) catch |err| switch (err) {
|
||||
error.EndOfStream => return total,
|
||||
error.WriteFailed => return error.Unexpected,
|
||||
else => |e| return mapError(e, .receive),
|
||||
error.WriteFailed => return out.fail(err, error.Unexpected),
|
||||
else => |e| return out.fail(e, mapError(e, .receive_body)),
|
||||
};
|
||||
return if (extra == 0) total else error.BodyTooLarge;
|
||||
return if (extra == 0) total else out.fail(error.BodyTooLarge, error.BodyTooLarge);
|
||||
}
|
||||
|
||||
/// A scheme other than `http`/`https`, an unparseable URL and a URL with no
|
||||
@@ -151,11 +261,6 @@ fn parseUrl(url: []const u8) Error!std.Uri {
|
||||
return uri;
|
||||
}
|
||||
|
||||
/// Which call failed. The phase is what decides the classification, and only
|
||||
/// the call site knows it — guessing it from an error name would be wrong the
|
||||
/// first time two phases shared an error.
|
||||
const Phase = enum { connect, send, receive };
|
||||
|
||||
// `error.X` in an expression names a member into existence rather than
|
||||
// referring to one, so the switch in `mapError` would keep compiling — and
|
||||
// silently stop matching — if std renamed either of these. This is what fails
|
||||
@@ -179,9 +284,10 @@ fn errorSetHas(comptime Set: type, comptime name: []const u8) bool {
|
||||
/// `std.http.Client` collapses every handshake fault into
|
||||
/// `error.TlsInitializationFailed` (Client.zig:1470) and every bundle fault into
|
||||
/// `error.CertificateBundleLoadFailure`, and this file never unwraps a
|
||||
/// connection's stashed read cause — it maps the collapsed `error.ReadFailed` by
|
||||
/// phase — so the record-layer members of `std.crypto.tls.Client.ReadError`
|
||||
/// cannot arrive here. `doh_client.zig` does unwrap, and names them.
|
||||
/// connection's stashed read cause *for classification* — it maps the collapsed
|
||||
/// `error.ReadFailed` by phase — so the record-layer members of
|
||||
/// `std.crypto.tls.Client.ReadError` cannot arrive here. `fetch` does unwrap
|
||||
/// them, into `Failure.cause`, which no classification reads.
|
||||
fn mapError(err: anyerror, phase: Phase) Error {
|
||||
if (transport.mapLocal(err)) |local| return narrowLocal(local);
|
||||
switch (err) {
|
||||
@@ -198,7 +304,7 @@ fn mapError(err: anyerror, phase: Phase) Error {
|
||||
return switch (phase) {
|
||||
.connect => error.ConnectFailed,
|
||||
.send => error.SendFailed,
|
||||
.receive => error.ReceiveFailed,
|
||||
.receive, .receive_body => error.ReceiveFailed,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -230,7 +336,8 @@ test "pumpBody streams a fully buffered body without aliasing its source" {
|
||||
var out: [payload.len]u8 = undefined;
|
||||
var sink: std.Io.Writer = .fixed(&out);
|
||||
|
||||
const n = try pumpBody(&body, &sink, max_body_bytes);
|
||||
var pump: Pump = .{};
|
||||
const n = try pumpBody(&body, &sink, max_body_bytes, &pump);
|
||||
try testing.expectEqual(@as(u64, payload.len), n);
|
||||
try testing.expectEqualStrings(payload, out[0..payload.len]);
|
||||
}
|
||||
@@ -241,7 +348,8 @@ test "pumpBody accepts a body of exactly the cap" {
|
||||
var out: [payload.len]u8 = undefined;
|
||||
var sink: std.Io.Writer = .fixed(&out);
|
||||
|
||||
const n = try pumpBody(&body, &sink, payload.len);
|
||||
var pump: Pump = .{};
|
||||
const n = try pumpBody(&body, &sink, payload.len, &pump);
|
||||
try testing.expectEqual(@as(u64, payload.len), n);
|
||||
try testing.expectEqualStrings(payload, out[0..payload.len]);
|
||||
}
|
||||
@@ -252,10 +360,12 @@ test "pumpBody refuses a body one byte over the cap" {
|
||||
var out: [payload.len]u8 = undefined;
|
||||
var sink: std.Io.Writer = .fixed(&out);
|
||||
|
||||
var pump: Pump = .{};
|
||||
try testing.expectError(
|
||||
error.BodyTooLarge,
|
||||
pumpBody(&body, &sink, payload.len - 1),
|
||||
pumpBody(&body, &sink, payload.len - 1, &pump),
|
||||
);
|
||||
try testing.expectEqual(error.BodyTooLarge, pump.cause);
|
||||
}
|
||||
|
||||
test "pumpBody reports an empty body as zero bytes" {
|
||||
@@ -263,7 +373,8 @@ test "pumpBody reports an empty body as zero bytes" {
|
||||
var sink_buf: [0]u8 = .{};
|
||||
var discarding: std.Io.Writer.Discarding = .init(&sink_buf);
|
||||
|
||||
try testing.expectEqual(@as(u64, 0), try pumpBody(&body, &discarding.writer, max_body_bytes));
|
||||
var pump: Pump = .{};
|
||||
try testing.expectEqual(@as(u64, 0), try pumpBody(&body, &discarding.writer, max_body_bytes, &pump));
|
||||
}
|
||||
|
||||
fn undefinedFetcher(transfer_buf: []u8, redirect_buf: []u8) Fetcher {
|
||||
@@ -359,3 +470,171 @@ test "caps are the values the memory budget was sized against" {
|
||||
try testing.expectEqual(@as(usize, 64 * 1024 * 1024), max_body_bytes);
|
||||
try testing.expectEqual(@as(usize, 8192), redirect_buffer_len);
|
||||
}
|
||||
|
||||
/// Only the fields the unwrap helpers read are set; the rest of a `Connection`
|
||||
/// is two buffered streams, a host name and a pool node. `.plain` for the
|
||||
/// reason `doh_client.zig` gives: `getReadError` reaches a TLS connection's
|
||||
/// cause through `@fieldParentPtr`, which on a stub would read memory that was
|
||||
/// never a `Tls`.
|
||||
fn stubConnection(read_err: ?std.Io.net.Stream.Reader.Error) std.http.Client.Connection {
|
||||
var connection: std.http.Client.Connection = undefined;
|
||||
connection.protocol = .plain;
|
||||
connection.stream_reader.err = read_err;
|
||||
connection.stream_writer.err = null;
|
||||
return connection;
|
||||
}
|
||||
|
||||
test "a head failure records the stashed cause, not the collapsed read error" {
|
||||
var connection = stubConnection(error.ConnectionResetByPeer);
|
||||
var req: std.http.Client.Request = undefined;
|
||||
req.connection = &connection;
|
||||
|
||||
var transfer_buf: [min_transfer_buf]u8 = undefined;
|
||||
var redirect_buf: [redirect_buffer_len]u8 = undefined;
|
||||
var f = undefinedFetcher(&transfer_buf, &redirect_buf);
|
||||
|
||||
const err = error.ReadFailed;
|
||||
const mapped = f.record(mapError(err, .receive), .receive, transport.headCause(&req, err), null, 0);
|
||||
|
||||
// The taxonomy the caller sees is what it was before the unwrap existed.
|
||||
try testing.expectEqual(error.ReceiveFailed, mapped);
|
||||
const failure = f.last_failure.?;
|
||||
try testing.expectEqual(Phase.receive, failure.phase);
|
||||
try testing.expectEqual(error.ConnectionResetByPeer, failure.cause);
|
||||
try testing.expectEqual(@as(?std.http.Status, null), failure.status);
|
||||
try testing.expectEqual(@as(u64, 0), failure.bytes_read);
|
||||
}
|
||||
|
||||
test "a body failure records the bytes delivered before it and the body phase" {
|
||||
const payload = "0.0.0.0 ads.example.com\n";
|
||||
var buffer: [payload.len]u8 = (payload ++ "").*;
|
||||
// Buffered bytes reach `w` first; the vtable is only asked for more once
|
||||
// the buffer is drained, which is the shape of a transfer cut off mid-body.
|
||||
var body: std.Io.Reader = .{
|
||||
.vtable = std.Io.Reader.failing.vtable,
|
||||
.buffer = &buffer,
|
||||
.seek = 0,
|
||||
.end = buffer.len,
|
||||
};
|
||||
var sink_buf: [0]u8 = .{};
|
||||
var discarding: std.Io.Writer.Discarding = .init(&sink_buf);
|
||||
|
||||
var pump: Pump = .{};
|
||||
try testing.expectError(
|
||||
error.ReceiveFailed,
|
||||
pumpBody(&body, &discarding.writer, max_body_bytes, &pump),
|
||||
);
|
||||
try testing.expectEqual(@as(u64, payload.len), pump.delivered);
|
||||
|
||||
var connection = stubConnection(null);
|
||||
var req: std.http.Client.Request = undefined;
|
||||
req.connection = &connection;
|
||||
req.reader.body_err = error.HttpChunkTruncated;
|
||||
const resp: std.http.Client.Response = .{ .request = &req, .head = undefined };
|
||||
|
||||
var transfer_buf: [min_transfer_buf]u8 = undefined;
|
||||
var redirect_buf: [redirect_buffer_len]u8 = undefined;
|
||||
var f = undefinedFetcher(&transfer_buf, &redirect_buf);
|
||||
const mapped = f.record(
|
||||
mapError(error.ReadFailed, .receive_body),
|
||||
.receive_body,
|
||||
transport.bodyCause(&resp, pump.cause),
|
||||
.ok,
|
||||
pump.delivered,
|
||||
);
|
||||
|
||||
try testing.expectEqual(error.ReceiveFailed, mapped);
|
||||
const failure = f.last_failure.?;
|
||||
try testing.expectEqual(Phase.receive_body, failure.phase);
|
||||
try testing.expectEqual(error.HttpChunkTruncated, failure.cause);
|
||||
try testing.expectEqual(@as(?std.http.Status, .ok), failure.status);
|
||||
try testing.expectEqual(@as(u64, payload.len), failure.bytes_read);
|
||||
}
|
||||
|
||||
test "a rejected url records the phase without claiming a connect failure" {
|
||||
var transfer_buf: [min_transfer_buf]u8 = undefined;
|
||||
var redirect_buf: [redirect_buffer_len]u8 = undefined;
|
||||
var f = undefinedFetcher(&transfer_buf, &redirect_buf);
|
||||
var sink_buf: [0]u8 = .{};
|
||||
var discarding: std.Io.Writer.Discarding = .init(&sink_buf);
|
||||
|
||||
try testing.expectError(
|
||||
error.BadUrl,
|
||||
f.fetch(undefined, "ftp://example.com/list.txt", &discarding.writer),
|
||||
);
|
||||
try testing.expectEqual(error.BadUrl, f.last_failure.?.cause);
|
||||
}
|
||||
|
||||
test "record returns the taxonomy member the call site raised, not a remapped one" {
|
||||
// `error.HttpStatus` and `error.BodyTooLarge` are raised by `fetch` itself
|
||||
// rather than by std, so `mapError` has no case for either and would fall
|
||||
// to the phase default. `record` must not run them through it.
|
||||
var transfer_buf: [min_transfer_buf]u8 = undefined;
|
||||
var redirect_buf: [redirect_buffer_len]u8 = undefined;
|
||||
var f = undefinedFetcher(&transfer_buf, &redirect_buf);
|
||||
|
||||
try testing.expectEqual(
|
||||
error.HttpStatus,
|
||||
f.record(error.HttpStatus, .receive, error.HttpStatus, .not_found, 0),
|
||||
);
|
||||
try testing.expectEqual(@as(?std.http.Status, .not_found), f.last_failure.?.status);
|
||||
try testing.expectEqual(error.HttpStatus, f.last_failure.?.cause);
|
||||
|
||||
try testing.expectEqual(
|
||||
error.BodyTooLarge,
|
||||
f.record(error.BodyTooLarge, .receive, error.BodyTooLarge, .ok, 0),
|
||||
);
|
||||
try testing.expectEqual(error.BodyTooLarge, f.last_failure.?.cause);
|
||||
|
||||
// This is why: `mapError` is unchanged and does collapse both to the phase
|
||||
// default. `fetch` must never route these two through it.
|
||||
try testing.expectEqual(error.ReceiveFailed, mapError(error.HttpStatus, .receive));
|
||||
try testing.expectEqual(error.ReceiveFailed, mapError(error.BodyTooLarge, .receive));
|
||||
}
|
||||
|
||||
test "a cancelled body read records the cancellation and the progress so far" {
|
||||
// This is the shape the expiry race leaves behind. `Select.cancelDiscard`
|
||||
// cancels the fetch, the cancellation surfaces from the socket as a
|
||||
// stashed `error.Canceled` under a collapsed `error.ReadFailed`, and the
|
||||
// fetch records it on its way out. The manager then reports its own
|
||||
// `error.Timeout` beside this record.
|
||||
const payload = "0.0.0.0 ads.example.com\n";
|
||||
var buffer: [payload.len]u8 = (payload ++ "").*;
|
||||
var body: std.Io.Reader = .{
|
||||
.vtable = std.Io.Reader.failing.vtable,
|
||||
.buffer = &buffer,
|
||||
.seek = 0,
|
||||
.end = buffer.len,
|
||||
};
|
||||
var sink_buf: [0]u8 = .{};
|
||||
var discarding: std.Io.Writer.Discarding = .init(&sink_buf);
|
||||
|
||||
var pump: Pump = .{};
|
||||
try testing.expectError(
|
||||
error.ReceiveFailed,
|
||||
pumpBody(&body, &discarding.writer, max_body_bytes, &pump),
|
||||
);
|
||||
|
||||
var connection = stubConnection(error.Canceled);
|
||||
var req: std.http.Client.Request = undefined;
|
||||
req.connection = &connection;
|
||||
req.reader.body_err = null;
|
||||
const resp: std.http.Client.Response = .{ .request = &req, .head = undefined };
|
||||
|
||||
var transfer_buf: [min_transfer_buf]u8 = undefined;
|
||||
var redirect_buf: [redirect_buffer_len]u8 = undefined;
|
||||
var f = undefinedFetcher(&transfer_buf, &redirect_buf);
|
||||
// `pumpBody` maps the collapsed `error.ReadFailed` it was handed, not the
|
||||
// unwrapped cause, so the taxonomy stays `ReceiveFailed` here. That is what
|
||||
// this path returned before `Failure` existed, and the addendum keeps it:
|
||||
// only the recorded cause is new. The manager reports `error.Timeout` from
|
||||
// the expiry race anyway, so nothing reads this return value on this path.
|
||||
const cause = transport.bodyCause(&resp, error.ReadFailed);
|
||||
const mapped = f.record(error.ReceiveFailed, .receive_body, cause, .ok, pump.delivered);
|
||||
|
||||
try testing.expectEqual(error.ReceiveFailed, mapped);
|
||||
try testing.expectEqual(Phase.receive_body, f.last_failure.?.phase);
|
||||
try testing.expectEqual(error.Canceled, f.last_failure.?.cause);
|
||||
try testing.expectEqual(@as(?std.http.Status, .ok), f.last_failure.?.status);
|
||||
try testing.expectEqual(@as(u64, payload.len), f.last_failure.?.bytes_read);
|
||||
}
|
||||
|
||||
@@ -421,7 +421,7 @@ const Env = struct {
|
||||
// fixtures: the loopback http server
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
const Route = enum(u8) { body, redirect, not_found, oversize, chunked, stall, changed };
|
||||
const Route = enum(u8) { body, redirect, not_found, oversize, chunked, stall, changed, stale_keep_alive };
|
||||
|
||||
/// How long the `stall` route holds a reply open when nothing releases it.
|
||||
///
|
||||
@@ -542,9 +542,10 @@ const HttpFixture = struct {
|
||||
}
|
||||
}
|
||||
|
||||
/// Every reply closes the connection. A keep-alive reply would leave the
|
||||
/// fetcher holding the connection open while this server waits to accept a
|
||||
/// second one that never comes (milestone-5 spec, S9 note from S8).
|
||||
/// Every reply but `stale_keep_alive` closes the connection. A keep-alive
|
||||
/// reply would leave the fetcher holding the connection open while this
|
||||
/// server waits to accept a second one that never comes (milestone-5 spec,
|
||||
/// S9 note from S8).
|
||||
fn respond(self: *HttpFixture, io: std.Io, request: *std.http.Server.Request) !void {
|
||||
switch (@as(Route, @enumFromInt(self.route.load(.acquire)))) {
|
||||
.body => try request.respond(self.body, .{ .keep_alive = false }),
|
||||
@@ -566,6 +567,11 @@ const HttpFixture = struct {
|
||||
.transfer_encoding = .none,
|
||||
.extra_headers = &.{.{ .name = "content-length", .value = oversize_length }},
|
||||
}),
|
||||
// Advertises keep-alive and then closes anyway, because `serve`
|
||||
// takes one request per connection. That is the Pi's failure in
|
||||
// miniature: a client that pools this connection reuses one the
|
||||
// peer has already closed on its next request to the host.
|
||||
.stale_keep_alive => try request.respond(self.body, .{ .keep_alive = true }),
|
||||
.chunked => try self.respondChunked(request),
|
||||
.stall => try self.respondStalled(io, request),
|
||||
}
|
||||
@@ -1003,6 +1009,19 @@ test "5: a redirect is followed to the same result" {
|
||||
try testing.expect(decision.blocked);
|
||||
}
|
||||
|
||||
/// The download failure line ends in numbers no test can predict — an elapsed
|
||||
/// time always, and a byte count where the failure is a cancelled transfer. The
|
||||
/// fixed part is compared whole; the tail only has to be digits, spaces and the
|
||||
/// two words those numbers carry.
|
||||
fn expectFailureText(expected_prefix: []const u8, actual: []const u8) !void {
|
||||
try testing.expectEqualStrings(expected_prefix, actual[0..@min(expected_prefix.len, actual.len)]);
|
||||
const tail = actual[expected_prefix.len..];
|
||||
try testing.expect(std.mem.endsWith(u8, tail, " ms)"));
|
||||
for (tail[0 .. tail.len - " ms)".len]) |c| {
|
||||
try testing.expect(std.ascii.isDigit(c) or c == ' ' or c == ',' or std.ascii.isAlphabetic(c));
|
||||
}
|
||||
}
|
||||
|
||||
test "6: a 404 leaves the compiled files and the snapshot untouched" {
|
||||
if (!build_options.integration) return error.SkipZigTest;
|
||||
|
||||
@@ -1034,7 +1053,13 @@ test "6: a 404 leaves the compiled files and the snapshot untouched" {
|
||||
|
||||
const failed = try env.status(id);
|
||||
try testing.expectEqual(manager.State.fetch_failed, failed.state);
|
||||
try testing.expectEqualStrings("HttpStatus", failed.errorText());
|
||||
// The whole line, not just the taxonomy: a 404 must stay `HttpStatus` and
|
||||
// name the status it saw. Routing it back through `fetcher.mapError` would
|
||||
// read `ReceiveFailed (receive, cause ReceiveFailed, ...)` here.
|
||||
try expectFailureText(
|
||||
"HttpStatus (receive, cause HttpStatus, http 404, 0 bytes, ",
|
||||
failed.errorText(),
|
||||
);
|
||||
|
||||
var after = try Bodies.read(gpa, io, dir, "1");
|
||||
defer after.deinit(gpa);
|
||||
@@ -1046,6 +1071,51 @@ test "6: a 404 leaves the compiled files and the snapshot untouched" {
|
||||
try testing.expect(decision.blocked);
|
||||
}
|
||||
|
||||
test "6b: a local failure reports itself, not the previous source's network fault" {
|
||||
if (!build_options.integration) return error.SkipZigTest;
|
||||
// Mode bits do not apply to root, so the denial the test needs cannot happen.
|
||||
if (std.c.geteuid() == 0) return error.SkipZigTest;
|
||||
|
||||
const gpa = testing.allocator;
|
||||
const env = try Env.create(gpa);
|
||||
defer env.destroy();
|
||||
const io = env.io();
|
||||
|
||||
var fixture = try HttpFixture.init(io, http_body);
|
||||
defer fixture.deinit(io);
|
||||
fixture.setRoute(.not_found);
|
||||
var group: std.Io.Group = .init;
|
||||
defer group.cancel(io);
|
||||
try group.concurrent(io, HttpFixture.serve, .{ &fixture, io });
|
||||
|
||||
var url_buf: [64]u8 = undefined;
|
||||
const url = try fixture.url(&url_buf);
|
||||
const id = try seedSource(&env.database, url);
|
||||
|
||||
// First a real network failure, so the fetcher is holding a record.
|
||||
try testing.expect(!try refreshOnce(env, url));
|
||||
try expectFailureText(
|
||||
"HttpStatus (receive, cause HttpStatus, http 404, 0 bytes, ",
|
||||
(try env.status(id)).errorText(),
|
||||
);
|
||||
|
||||
// Then a failure that never reaches the network: the raw file cannot be
|
||||
// created. Without the clear at the top of `download` this would report the
|
||||
// 404 above as this refresh's cause.
|
||||
var dir = try env.blocklistDir();
|
||||
defer dir.close(io);
|
||||
try dir.setPermissions(io, .fromMode(0o500));
|
||||
defer dir.setPermissions(io, .fromMode(0o700)) catch {};
|
||||
|
||||
try testing.expect(!try refreshOnce(env, url));
|
||||
|
||||
const failed = try env.status(id);
|
||||
try testing.expectEqual(manager.State.fetch_failed, failed.state);
|
||||
try expectFailureText("AccessDenied (no fetch, http none, - bytes, ", failed.errorText());
|
||||
try testing.expect(!std.mem.containsAtLeast(u8, failed.errorText(), 1, "HttpStatus"));
|
||||
try testing.expect(!std.mem.containsAtLeast(u8, failed.errorText(), 1, "404"));
|
||||
}
|
||||
|
||||
test "7: a body over the cap fails the refresh and leaves no temporary file" {
|
||||
if (!build_options.integration) return error.SkipZigTest;
|
||||
|
||||
@@ -1069,7 +1139,10 @@ test "7: a body over the cap fails the refresh and leaves no temporary file" {
|
||||
|
||||
const failed = try env.status(id);
|
||||
try testing.expectEqual(manager.State.fetch_failed, failed.state);
|
||||
try testing.expectEqualStrings("BodyTooLarge", failed.errorText());
|
||||
try expectFailureText(
|
||||
"BodyTooLarge (receive, cause BodyTooLarge, http 200, 0 bytes, ",
|
||||
failed.errorText(),
|
||||
);
|
||||
|
||||
var dir = try env.blocklistDir();
|
||||
defer dir.close(io);
|
||||
@@ -1187,6 +1260,44 @@ fn publishFixtureFiles(env: *Env, id: i64, list_body: []const u8, wild_body: []c
|
||||
});
|
||||
}
|
||||
|
||||
test "8b: a second download to the same host never reuses a dead pooled connection" {
|
||||
if (!build_options.integration) return error.SkipZigTest;
|
||||
|
||||
const gpa = testing.allocator;
|
||||
const env = try Env.create(gpa);
|
||||
defer env.destroy();
|
||||
const io = env.io();
|
||||
|
||||
var fixture = try HttpFixture.init(io, http_body);
|
||||
defer fixture.deinit(io);
|
||||
// The server says keep-alive and then drops the socket, which is what the
|
||||
// Pi's upstreams do between one daily pass and the next.
|
||||
fixture.setRoute(.stale_keep_alive);
|
||||
var group: std.Io.Group = .init;
|
||||
defer group.cancel(io);
|
||||
try group.concurrent(io, HttpFixture.serve, .{ &fixture, io });
|
||||
|
||||
var url_buf: [64]u8 = undefined;
|
||||
const url = try fixture.url(&url_buf);
|
||||
|
||||
var sink_buf: [0]u8 = .{};
|
||||
var first: std.Io.Writer.Discarding = .init(&sink_buf);
|
||||
const one = try env.f.fetch(io, url, &first.writer);
|
||||
try testing.expectEqual(@as(u64, http_body.len), one.bytes_read);
|
||||
|
||||
// The whole test is this second call. With a pooling client the request
|
||||
// goes out over a connection the peer has already closed, no response head
|
||||
// byte arrives, and `error.HttpConnectionClosing` comes back with no retry
|
||||
// — the exact record the Pi produced at 0 bytes and under a millisecond.
|
||||
var second: std.Io.Writer.Discarding = .init(&sink_buf);
|
||||
const two = try env.f.fetch(io, url, &second.writer);
|
||||
try testing.expectEqual(@as(u64, http_body.len), two.bytes_read);
|
||||
try testing.expectEqual(@as(?fetcher.Failure, null), env.f.last_failure);
|
||||
|
||||
// Two connections, not one reused: the proof the first was not pooled.
|
||||
try testing.expectEqual(@as(u32, 2), fixture.accepted.load(.monotonic));
|
||||
}
|
||||
|
||||
test "9: a reload swaps under a held handle and the new generation follows the release" {
|
||||
if (!build_options.integration) return error.SkipZigTest;
|
||||
|
||||
|
||||
+196
-15
@@ -69,8 +69,13 @@ const log = std.log.scoped(.blocklist_manager);
|
||||
const Sha256 = std.crypto.hash.sha2.Sha256;
|
||||
|
||||
/// `SourceStatus.last_error` is fixed-size so the failure path allocates
|
||||
/// nothing.
|
||||
pub const max_error_len: usize = 128;
|
||||
/// nothing. Sized so the longest `downloadFailureText` fits whole: the widest
|
||||
/// cause name the unwraps can produce is `DetectingNetworkConfigurationFailed`
|
||||
/// at 35 bytes, which with a 64 MiB byte count and an hour of milliseconds
|
||||
/// makes a 111-byte line. The test below recomputes that worst case from the
|
||||
/// std error sets, so a wider name std adds fails the build's test run rather
|
||||
/// than silently truncating an operator's only diagnostic.
|
||||
pub const max_error_len: usize = 192;
|
||||
|
||||
/// `SourceStatus.url` is fixed-size so a copied status borrows nothing. A
|
||||
/// blocklist url longer than this is truncated in the status only; the row
|
||||
@@ -131,6 +136,55 @@ const SourceLabel = struct {
|
||||
}
|
||||
};
|
||||
|
||||
/// The one place the download failure text is formatted. `phase`, `cause` and
|
||||
/// the byte count come from the fetcher's record of the concrete fault.
|
||||
///
|
||||
/// A timeout carries a record like any other failure: `fetchWithin` cancels the
|
||||
/// fetch on its way out of the race, and the cancelled fetch records the phase
|
||||
/// it was in, the status if a head had arrived and the bytes delivered so far,
|
||||
/// all before this function reads it. So an expiry reads
|
||||
/// `Timeout (receive_body, cause Canceled, http 200, 5242880 bytes, 300000 ms)`.
|
||||
/// `no fetch` and `-` are for a failure that never reached the network at all:
|
||||
/// `download` clears the record before anything can fail, so a local fault —
|
||||
/// the temporary file, an allocation, a refused task — reads as its own
|
||||
/// taxonomy name with nothing borrowed from the source fetched before it.
|
||||
fn downloadFailureText(
|
||||
buf: []u8,
|
||||
err: anyerror,
|
||||
failure: ?fetcher.Failure,
|
||||
last_status: ?std.http.Status,
|
||||
elapsed_ms: u64,
|
||||
) []const u8 {
|
||||
var status_buf: [8]u8 = undefined;
|
||||
const status_text = if (if (failure) |f| f.status else last_status) |code|
|
||||
std.fmt.bufPrint(&status_buf, "{d}", .{@intFromEnum(code)}) catch "none"
|
||||
else
|
||||
"none";
|
||||
|
||||
var bytes_buf: [24]u8 = undefined;
|
||||
const bytes_text = if (failure) |f|
|
||||
std.fmt.bufPrint(&bytes_buf, "{d}", .{f.bytes_read}) catch "-"
|
||||
else
|
||||
"-";
|
||||
|
||||
if (failure) |f| {
|
||||
return std.fmt.bufPrint(buf, "{s} ({s}, cause {s}, http {s}, {s} bytes, {d} ms)", .{
|
||||
@errorName(err),
|
||||
@tagName(f.phase),
|
||||
@errorName(f.cause),
|
||||
status_text,
|
||||
bytes_text,
|
||||
elapsed_ms,
|
||||
}) catch @errorName(err);
|
||||
}
|
||||
return std.fmt.bufPrint(buf, "{s} (no fetch, http {s}, {s} bytes, {d} ms)", .{
|
||||
@errorName(err),
|
||||
status_text,
|
||||
bytes_text,
|
||||
elapsed_ms,
|
||||
}) catch @errorName(err);
|
||||
}
|
||||
|
||||
pub const Paths = struct {
|
||||
/// `<data_dir>`, owned by the caller and left open for the manager's life.
|
||||
dir: std.Io.Dir,
|
||||
@@ -1169,11 +1223,12 @@ pub const Manager = struct {
|
||||
raw_name: []const u8,
|
||||
tmp: TempNames,
|
||||
) Error!Prepared {
|
||||
self.download(io, dir, raw_name, row) catch |err| switch (err) {
|
||||
var elapsed_ms: u64 = 0;
|
||||
self.download(io, dir, raw_name, row, &elapsed_ms) catch |err| switch (err) {
|
||||
error.OutOfMemory => return error.OutOfMemory,
|
||||
error.Canceled => return error.Canceled,
|
||||
else => {
|
||||
self.reportFetchFailure(row, status, err);
|
||||
self.reportDownloadFailure(row, status, err, elapsed_ms);
|
||||
return .failed;
|
||||
},
|
||||
};
|
||||
@@ -1297,7 +1352,18 @@ pub const Manager = struct {
|
||||
dir: std.Io.Dir,
|
||||
raw_name: []const u8,
|
||||
row: sources_repo.SourceRow,
|
||||
/// Milliseconds the download itself took, set whether it succeeded or
|
||||
/// failed, so the failure line can say how long the peer had.
|
||||
elapsed_ms: *u64,
|
||||
) !void {
|
||||
// Ahead of everything that can fail. The record is the fetcher's, not
|
||||
// this source's, and a local failure here — the create, the allocation,
|
||||
// a refused task — would otherwise leave the PREVIOUS source's network
|
||||
// cause in place for `reportDownloadFailure` to print as this one's.
|
||||
// Cleared here, a null record means no fetch reached the network.
|
||||
self.fetcher.last_failure = null;
|
||||
self.fetcher.last_status = null;
|
||||
|
||||
const file = try dir.createFile(io, raw_name, .{ .permissions = .fromMode(0o600) });
|
||||
defer file.close(io);
|
||||
|
||||
@@ -1305,16 +1371,14 @@ pub const Manager = struct {
|
||||
defer self.gpa.free(buffer);
|
||||
|
||||
var fw = file.writer(io, buffer);
|
||||
const result = self.fetchWithin(io, row.url, &fw.interface) catch |err| {
|
||||
const result = self.fetchWithin(io, row.url, &fw.interface, elapsed_ms) catch |err| {
|
||||
// `fetcher.Error.Unexpected` is what a failing sink surfaces as;
|
||||
// the concrete cause is on this writer, which the fetcher does not
|
||||
// own.
|
||||
if (fw.err) |cause| return cause;
|
||||
if (err == error.HttpStatus) {
|
||||
if (self.fetcher.last_status) |status| {
|
||||
log.warn("blocklist {f}: http status {d}", .{ SourceLabel.of(row), @intFromEnum(status) });
|
||||
}
|
||||
}
|
||||
// The status used to get a second warning of its own. It is a field
|
||||
// of the one download-failure line now, so a second line would only
|
||||
// repeat it.
|
||||
return err;
|
||||
};
|
||||
try fw.interface.flush();
|
||||
@@ -1332,7 +1396,12 @@ pub const Manager = struct {
|
||||
io: std.Io,
|
||||
url: []const u8,
|
||||
w: *std.Io.Writer,
|
||||
elapsed_ms: *u64,
|
||||
) fetcher.Error!fetcher.Result {
|
||||
// `awake` and not `real`: an operator reading "812 ms" wants the time
|
||||
// the transfer was given, which a stepped wall clock would misreport.
|
||||
const started = std.Io.Clock.awake.now(io);
|
||||
|
||||
var outcomes: [2]Outcome = undefined;
|
||||
var race: std.Io.Select(Outcome) = .init(io, &outcomes);
|
||||
defer race.cancelDiscard();
|
||||
@@ -1344,7 +1413,12 @@ pub const Manager = struct {
|
||||
error.ConcurrencyUnavailable => return error.SystemResources,
|
||||
};
|
||||
|
||||
switch (try race.await()) {
|
||||
// Read before the deferred `cancelDiscard`, which tears the loser down:
|
||||
// the number is the time the peer had, not that plus the teardown.
|
||||
const outcome = race.await();
|
||||
elapsed_ms.* = @intCast(@max(0, started.durationTo(std.Io.Clock.awake.now(io)).toMilliseconds()));
|
||||
|
||||
switch (try outcome) {
|
||||
.fetch => |result| return result,
|
||||
.expiry => |result| {
|
||||
// A canceled sleep means this task is being torn down, not that
|
||||
@@ -1514,15 +1588,21 @@ pub const Manager = struct {
|
||||
return compiledBodiesMatch(list_bytes, wild_bytes, allow_bytes, expected);
|
||||
}
|
||||
|
||||
fn reportFetchFailure(
|
||||
/// One line per failed source per pass, and the only place the download
|
||||
/// failure text is built. The Diagnostics event detail gets the same text,
|
||||
/// so an operator reading either can tell a TLS alert from a reset
|
||||
/// connection from a truncated chunk.
|
||||
fn reportDownloadFailure(
|
||||
self: *Manager,
|
||||
row: sources_repo.SourceRow,
|
||||
status: *SourceStatus,
|
||||
err: anyerror,
|
||||
elapsed_ms: u64,
|
||||
) void {
|
||||
_ = self;
|
||||
log.warn("blocklist {f}: download failed: {s}", .{ SourceLabel.of(row), @errorName(err) });
|
||||
status.fail(.fetch_failed, @errorName(err));
|
||||
var buf: [max_error_len]u8 = undefined;
|
||||
const text = downloadFailureText(&buf, err, self.fetcher.last_failure, self.fetcher.last_status, elapsed_ms);
|
||||
log.warn("blocklist {f}: download failed: {s}", .{ SourceLabel.of(row), text });
|
||||
status.fail(.fetch_failed, text);
|
||||
}
|
||||
|
||||
fn reportCompileFailure(
|
||||
@@ -3580,3 +3660,104 @@ test "setSchedule is what the live schedule readers see" {
|
||||
try testing.expectEqual(@as(u16, 6), live.interval_hours);
|
||||
try testing.expect(manager.schedule_event.isSet());
|
||||
}
|
||||
|
||||
test "a download failure line names the concrete cause behind the taxonomy" {
|
||||
var buf: [1024]u8 = undefined;
|
||||
const row: sources_repo.SourceRow = .{
|
||||
.id = 3,
|
||||
.url = "https://lists.example/download/token/hunter2/hosts.txt?apikey=s3cr3t",
|
||||
.name = "ads",
|
||||
.enabled = true,
|
||||
.last_updated = null,
|
||||
.domain_count = 0,
|
||||
.wildcard_count = 0,
|
||||
.skipped_regex_count = 0,
|
||||
.skipped_unsupported_count = 0,
|
||||
.checksum = null,
|
||||
};
|
||||
var text_buf: [max_error_len]u8 = undefined;
|
||||
const text = downloadFailureText(&text_buf, error.ReceiveFailed, .{
|
||||
.phase = .receive_body,
|
||||
.cause = error.HttpChunkTruncated,
|
||||
.status = .ok,
|
||||
.bytes_read = 1024 * 1024,
|
||||
}, .ok, 812);
|
||||
|
||||
const printed = try std.fmt.bufPrint(&buf, "blocklist {f}: download failed: {s}", .{
|
||||
SourceLabel.of(row),
|
||||
text,
|
||||
});
|
||||
try testing.expectEqualStrings(
|
||||
"blocklist source 3 'ads' 'https://lists.example': download failed:" ++
|
||||
" ReceiveFailed (receive_body, cause HttpChunkTruncated, http 200, 1048576 bytes, 812 ms)",
|
||||
printed,
|
||||
);
|
||||
try testing.expect(!std.mem.containsAtLeast(u8, printed, 1, "hunter2"));
|
||||
try testing.expect(!std.mem.containsAtLeast(u8, printed, 1, "s3cr3t"));
|
||||
}
|
||||
|
||||
test "an expiry names the phase and the progress the cancelled fetch had made" {
|
||||
// `fetchWithin` cancels the fetch before it reports, so a timeout normally
|
||||
// does have a record: the cancellation the fetch saw, under the manager's
|
||||
// own `error.Timeout`.
|
||||
var text_buf: [max_error_len]u8 = undefined;
|
||||
try testing.expectEqualStrings(
|
||||
"Timeout (receive_body, cause Canceled, http 200, 5242880 bytes, 300000 ms)",
|
||||
downloadFailureText(&text_buf, error.Timeout, .{
|
||||
.phase = .receive_body,
|
||||
.cause = error.Canceled,
|
||||
.status = .ok,
|
||||
.bytes_read = 5 * 1024 * 1024,
|
||||
}, .ok, 300000),
|
||||
);
|
||||
}
|
||||
|
||||
test "a failure that never reached the network says so and borrows nothing" {
|
||||
// `download` clears the fetcher's record before anything can fail, so a
|
||||
// local fault reads as itself. Printing a phase and a cause here would be
|
||||
// printing the previous source's network fault against this source.
|
||||
var text_buf: [max_error_len]u8 = undefined;
|
||||
try testing.expectEqualStrings(
|
||||
"SystemResources (no fetch, http none, - bytes, 0 ms)",
|
||||
downloadFailureText(&text_buf, error.SystemResources, null, null, 0),
|
||||
);
|
||||
try testing.expectEqualStrings(
|
||||
"AccessDenied (no fetch, http none, - bytes, 3 ms)",
|
||||
downloadFailureText(&text_buf, error.AccessDenied, null, null, 3),
|
||||
);
|
||||
}
|
||||
|
||||
test "max_error_len holds the widest download failure line whole" {
|
||||
// `@errorName` of the unwrapped cause is the only unbounded-looking part.
|
||||
// The two sets below are every set the unwraps in `upstream/transport.zig`
|
||||
// can return a member of.
|
||||
const widest_cause = comptime blk: {
|
||||
var widest: []const u8 = "";
|
||||
for (@typeInfo(std.crypto.tls.Client.ReadError).error_set.?) |member| {
|
||||
if (member.name.len > widest.len) widest = member.name;
|
||||
}
|
||||
for (@typeInfo(std.http.Client.RequestError).error_set.?) |member| {
|
||||
if (member.name.len > widest.len) widest = member.name;
|
||||
}
|
||||
for (@typeInfo(std.http.Reader.BodyError).error_set.?) |member| {
|
||||
if (member.name.len > widest.len) widest = member.name;
|
||||
}
|
||||
break :blk widest;
|
||||
};
|
||||
|
||||
var buf: [max_error_len]u8 = undefined;
|
||||
const text = std.fmt.bufPrint(
|
||||
&buf,
|
||||
"{s} ({s}, cause {s}, http {d}, {d} bytes, {d} ms)",
|
||||
.{
|
||||
"SystemResources",
|
||||
@tagName(fetcher.Phase.receive_body),
|
||||
widest_cause,
|
||||
@as(u16, 599),
|
||||
@as(u64, fetcher.max_body_bytes),
|
||||
@as(u64, std.time.ms_per_hour),
|
||||
},
|
||||
) catch unreachable;
|
||||
try testing.expect(text.len <= max_error_len);
|
||||
try testing.expectEqualStrings("DetectingNetworkConfigurationFailed", widest_cause);
|
||||
}
|
||||
|
||||
@@ -226,43 +226,12 @@ const Connection = std.http.Client.Connection;
|
||||
const Request = std.http.Client.Request;
|
||||
const Response = std.http.Client.Response;
|
||||
|
||||
/// `std.Io.Writer` collapses every send failure to `error.WriteFailed` and
|
||||
/// stashes the cause on the connection's socket writer. Unwrapping it is what
|
||||
/// keeps `error.Canceled` and the local resource errors out of the peer fault
|
||||
/// group, exactly as `concreteWrite` does for DoT.
|
||||
fn sendCause(req: *const Request, err: anyerror) anyerror {
|
||||
if (err != error.WriteFailed) return err;
|
||||
const connection = req.connection orelse return err;
|
||||
return connection.stream_writer.err orelse err;
|
||||
}
|
||||
|
||||
/// `receiveHead` collapses a transport failure to `error.ReadFailed` and names
|
||||
/// `Connection.getReadError` as the accessor for the concrete cause.
|
||||
fn headCause(req: *const Request, err: anyerror) anyerror {
|
||||
if (err != error.ReadFailed) return err;
|
||||
const connection = req.connection orelse return err;
|
||||
return readCause(connection, err);
|
||||
}
|
||||
|
||||
/// A body read reports two different kinds of failure through the same
|
||||
/// `error.ReadFailed`. An HTTP framing fault lands on the response, and only a
|
||||
/// read that never reached the framing leaves the connection's cause, so the
|
||||
/// response is consulted first.
|
||||
fn bodyCause(resp: *const Response, err: anyerror) anyerror {
|
||||
if (err != error.ReadFailed) return err;
|
||||
if (resp.bodyErr()) |cause| return cause;
|
||||
const connection = resp.request.connection orelse return err;
|
||||
return readCause(connection, err);
|
||||
}
|
||||
|
||||
/// `Connection.getReadError` reads the socket reader's stashed cause with `.?`.
|
||||
/// On a plain connection that is its only source, so calling it with nothing
|
||||
/// stashed would panic rather than return null; the guard keeps this unwrap
|
||||
/// total on the path this client can reach without TLS.
|
||||
fn readCause(connection: *const Connection, err: anyerror) anyerror {
|
||||
if (connection.protocol == .plain and connection.stream_reader.err == null) return err;
|
||||
return connection.getReadError() orelse err;
|
||||
}
|
||||
// The unwraps live in `transport.zig` because the blocklist fetcher needs the
|
||||
// same concrete causes. They are aliased rather than qualified so the call
|
||||
// sites and the tests below read as they did when they were local.
|
||||
const sendCause = transport.sendCause;
|
||||
const headCause = transport.headCause;
|
||||
const bodyCause = transport.bodyCause;
|
||||
|
||||
/// RFC 8484 §6: the response media type is `application/dns-message`. The
|
||||
/// header may carry parameters (`; charset=…`) and the type is case-insensitive
|
||||
|
||||
@@ -419,6 +419,52 @@ pub const Client = struct {
|
||||
}
|
||||
};
|
||||
|
||||
/// The unwrap helpers below turn the single collapsed error `std.http.Client`
|
||||
/// reports into the concrete cause it stashed. Every HTTP caller in this tree
|
||||
/// uses them: the DoH client classifies by the unwrapped cause, the blocklist
|
||||
/// fetcher keeps its own classification and records the cause for the operator.
|
||||
const Connection = std.http.Client.Connection;
|
||||
const Request = std.http.Client.Request;
|
||||
const Response = std.http.Client.Response;
|
||||
|
||||
/// `std.Io.Writer` collapses every send failure to `error.WriteFailed` and
|
||||
/// stashes the cause on the connection's socket writer. Unwrapping it is what
|
||||
/// keeps `error.Canceled` and the local resource errors out of the peer fault
|
||||
/// group.
|
||||
pub fn sendCause(req: *const Request, err: anyerror) anyerror {
|
||||
if (err != error.WriteFailed) return err;
|
||||
const connection = req.connection orelse return err;
|
||||
return connection.stream_writer.err orelse err;
|
||||
}
|
||||
|
||||
/// `receiveHead` collapses a transport failure to `error.ReadFailed` and names
|
||||
/// `Connection.getReadError` as the accessor for the concrete cause.
|
||||
pub fn headCause(req: *const Request, err: anyerror) anyerror {
|
||||
if (err != error.ReadFailed) return err;
|
||||
const connection = req.connection orelse return err;
|
||||
return readCause(connection, err);
|
||||
}
|
||||
|
||||
/// A body read reports two different kinds of failure through the same
|
||||
/// `error.ReadFailed`. An HTTP framing fault lands on the response, and only a
|
||||
/// read that never reached the framing leaves the connection's cause, so the
|
||||
/// response is consulted first.
|
||||
pub fn bodyCause(resp: *const Response, err: anyerror) anyerror {
|
||||
if (err != error.ReadFailed) return err;
|
||||
if (resp.bodyErr()) |cause| return cause;
|
||||
const connection = resp.request.connection orelse return err;
|
||||
return readCause(connection, err);
|
||||
}
|
||||
|
||||
/// `Connection.getReadError` reads the socket reader's stashed cause with `.?`.
|
||||
/// On a plain connection that is its only source, so calling it with nothing
|
||||
/// stashed would panic rather than return null; the guard keeps this unwrap
|
||||
/// total on the path a plain connection can reach without TLS.
|
||||
pub fn readCause(connection: *const Connection, err: anyerror) anyerror {
|
||||
if (connection.protocol == .plain and connection.stream_reader.err == null) return err;
|
||||
return connection.getReadError() orelse err;
|
||||
}
|
||||
|
||||
pub const ValidateError = error{ BadResponse, ResponseMismatch };
|
||||
|
||||
/// RFC 9619: exactly one question on both sides. RFC 4343: names compare
|
||||
|
||||
Reference in New Issue
Block a user