Skip to content

Commit cd026cd

Browse files
ralyodioclaude
andauthored
fix(dns): say when the ending list did not load (#270)
Swallowing this was the quietest way to switch the whole namespace off. isOurs() gates every answer on the ending set, so an empty one makes the bridge say "not mine" to every name — and with upstreams configured, "not mine" means forward to the clearnet, which denies all of Moshpit by definition. The machine ends up with dig answering promptly, google.com resolving, resolvectl looking right, systemctl green, and every Moshpit name NXDOMAIN. Nothing in any log mentioned the list, because the fetch was wrapped in .catch(() => []) and an empty array is also what a registry with no endings would legitimately return. The two lines directly beneath it have always warned about missing upstreams. What we answer for is worth at least as much as what we forward to. So: report it, name the consequence rather than the detail — an operator can act on "every Moshpit name will be forwarded to the clearnet" in a way they cannot act on "0 endings loaded" — and print the count on the way up, so a healthy start is visibly different from a broken one. The branch had never been exercised because `start` reached past the injected deps.tlds to the module-level function while `enable` used the injectable one. Now it uses fetchTldsImpl, which is what made the four tests here possible. Found while chasing a resolver that answered nothing for the namespace: the first diagnosis was a slow warm-up, which was wrong. fetchTlds is awaited before the socket binds and returns 5674 endings in about a second. There was no race — the list had simply failed to load, and nothing said so. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 27c2fb7 commit cd026cd

3 files changed

Lines changed: 132 additions & 2 deletions

File tree

src/dns.mjs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2119,9 +2119,30 @@ export async function dnsCommand(args = [], out = console.log, deps = {}) {
21192119
// Without these the bridge answers only for endings it is authoritative
21202120
// for, which is correct for per-ending routing and fatal for catch-all.
21212121
const upstreams = await discoverUpstreams();
2122-
const tldSet = new Set(await fetchTlds({ registryBase }).catch(() => []));
2122+
// Swallowing this was the quietest way to turn the namespace off. An empty
2123+
// ending set makes isOurs() say no to every name, so with upstreams present
2124+
// the bridge forwards the whole of Moshpit to the clearnet, which denies it
2125+
// — every name NXDOMAIN, `dig` answering promptly, google.com fine, nothing
2126+
// in any log. The line below has always warned about missing upstreams; the
2127+
// list of what we answer for is worth at least as much.
2128+
// `enable` already takes this injected; `start` reached past it to the
2129+
// module, which is why the branch below had never been exercised.
2130+
const tlds = await fetchTldsImpl({ registryBase }).then(
2131+
(found) => ({ found }),
2132+
(err) => ({ error: err?.message || String(err) }),
2133+
);
2134+
const tldSet = new Set(tlds.found || []);
21232135
if (upstreams.length) out(`forwarding non-Moshpit lookups to ${upstreams.join(", ")}`);
21242136
else out("! no upstreams found in /etc/resolv.conf — this bridge can only answer Moshpit names");
2137+
if (tldSet.size) out(`answering for ${tldSet.size} endings`);
2138+
else {
2139+
out(`! could not read the ending list from ${registryBase}${tlds.error ? ` — ${tlds.error}` : ""}`);
2140+
// Named as the outcome rather than the cause: "no endings loaded" reads
2141+
// as a detail, and this is the whole namespace being off.
2142+
out(upstreams.length
2143+
? " every Moshpit name will be forwarded to the clearnet and answer NXDOMAIN until this is fixed"
2144+
: " this bridge has nothing to answer for and nothing to forward to");
2145+
}
21252146

21262147
// The same two error codes the parking server above already explains, on
21272148
// the port this command exists to bind. Without this they arrived as an

test/dns-start-bind-failure.test.mjs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ import net from "node:net";
2626
import { createServer, dnsCommand } from "../src/dns.mjs";
2727

2828
// Nothing here may touch the network. A registry that cannot connect is enough:
29-
// `dns start` already wraps its TLD fetch in .catch(() => []).
29+
// `dns start` reports the unreadable ending list and carries on to the bind,
30+
// which is the failure these tests are about.
3031
const DEAD_REGISTRY = "http://127.0.0.1:1";
3132

3233
/** Hold a UDP port so the resolver's bind is guaranteed to fail. */

test/dns-tld-list-failure.test.mjs

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/**
2+
* What `dns start` says when it could not read the list of endings.
3+
*
4+
* This was the quietest way to switch the whole namespace off. `isOurs()` gates
5+
* every answer on the ending set, so an empty one makes the bridge say "not
6+
* mine" to every name — and with upstreams configured, "not mine" means forward
7+
* to the clearnet, which denies the entire Moshpit namespace by definition.
8+
*
9+
* The result is a machine where `dig` answers promptly, google.com resolves,
10+
* `resolvectl` looks right, `systemctl status` is green, and every Moshpit name
11+
* is NXDOMAIN. Nothing in any log said the list had not loaded, because the
12+
* fetch was wrapped in `.catch(() => [])` and the empty array is also what a
13+
* registry with no endings would legitimately return.
14+
*
15+
* The line immediately below it has always warned about missing upstreams. What
16+
* we answer for is worth at least as much as what we forward to.
17+
*
18+
* Every test drives the failure through a held port, so the command prints its
19+
* diagnosis and then returns rather than binding and serving forever.
20+
*/
21+
import test from "node:test";
22+
import assert from "node:assert/strict";
23+
import dgram from "node:dgram";
24+
25+
import { dnsCommand } from "../src/dns.mjs";
26+
27+
const DEAD_REGISTRY = "http://127.0.0.1:1";
28+
29+
/** Hold a UDP port so the resolver bind fails and `start` returns. */
30+
function holdUdp() {
31+
const socket = dgram.createSocket({ type: "udp4" });
32+
return new Promise((resolve) => {
33+
socket.bind(0, "127.0.0.1", () => resolve({
34+
port: socket.address().port,
35+
release: () => new Promise((done) => socket.close(done)),
36+
}));
37+
});
38+
}
39+
40+
/** Run `dns start` with an injected ending list, on a port that cannot bind. */
41+
async function start(port, tlds) {
42+
const lines = [];
43+
const code = await dnsCommand(
44+
["start", "--port", String(port), "--parking-port", String(port), "--registry", DEAD_REGISTRY],
45+
(line) => lines.push(line),
46+
{ tlds },
47+
);
48+
return { code, out: lines.join("\n") };
49+
}
50+
51+
test("a TLD list that could not be fetched is reported, with the cost named", async () => {
52+
const held = await holdUdp();
53+
try {
54+
const { out } = await start(held.port, async () => {
55+
throw new Error("registry unreachable");
56+
});
57+
58+
assert.match(out, /could not read the ending list/);
59+
assert.match(out, /registry unreachable/, "the reason belongs in the message, not just the fact");
60+
// The consequence is the part an operator can act on. "no endings loaded"
61+
// reads as a detail; this is the namespace being off.
62+
assert.match(out, /every Moshpit name will be forwarded to the clearnet/);
63+
} finally {
64+
await held.release();
65+
}
66+
});
67+
68+
test("an empty TLD list is reported too, not just a thrown one", async () => {
69+
// A registry that answers with nothing is indistinguishable in effect from one
70+
// that does not answer, and it was the case the old `.catch(() => [])` could
71+
// never have caught: no error was ever thrown.
72+
const held = await holdUdp();
73+
try {
74+
const { out } = await start(held.port, async () => []);
75+
assert.match(out, /could not read the ending list/);
76+
} finally {
77+
await held.release();
78+
}
79+
});
80+
81+
test("a TLD list that loaded says so, so the quiet case is not the same as the broken one", async () => {
82+
const held = await holdUdp();
83+
try {
84+
const { out } = await start(held.port, async () => ["eggs", "hacker", "rank"]);
85+
86+
assert.match(out, /answering for 3 endings/);
87+
assert.doesNotMatch(out, /could not read the ending list/);
88+
assert.doesNotMatch(out, /forwarded to the clearnet/);
89+
} finally {
90+
await held.release();
91+
}
92+
});
93+
94+
test("the ending list is taken from deps, the way `enable` already takes it", async () => {
95+
// `start` reached past the injected dependency to the module-level function,
96+
// which is why this whole branch had never been exercised by a test.
97+
const held = await holdUdp();
98+
let asked = 0;
99+
try {
100+
await start(held.port, async () => {
101+
asked += 1;
102+
return ["eggs"];
103+
});
104+
assert.equal(asked, 1, "the injected fetcher is the one that ran");
105+
} finally {
106+
await held.release();
107+
}
108+
});

0 commit comments

Comments
 (0)