Skip to content

Commit 2e288e4

Browse files
Claudesnichmeclaude
authored
Fix auth() to handle partial success for address families (#14)
* Initial plan * Fix auth() to use Promise.allSettled for dual-stack knock tolerance Replace Promise.all with Promise.allSettled in auth() so that a single unreachable address family (e.g. IPv6 ENETUNREACH) does not cause the entire auth call to fail when the knock to the other family succeeded. Now auth() only throws (with AggregateError) when ALL knocks fail. Also bump tsconfig target to ES2021 for AggregateError type support. Closes #13 Agent-Logs-Url: https://github.com/84codes/sparoid.js/sessions/c1d08905-5aa2-41cb-95f6-0515c412e206 Co-authored-by: snichme <102988+snichme@users.noreply.github.com> * Guard against empty results array in auth() knock check Agent-Logs-Url: https://github.com/84codes/sparoid.js/sessions/c1d08905-5aa2-41cb-95f6-0515c412e206 Co-authored-by: snichme <102988+snichme@users.noreply.github.com> * test: make knock-failure tests deterministic via socket mocking The "all knocks fail" and partial-success tests depended on real UDP/IPv6 behavior (sending to 100::1, dual-stack localhost), so their outcome varied with the host's IPv6 routing — flaky across environments (Copilot review on #14). Mock dgram.createSocket so send() outcomes are controlled directly: - all sends fail -> auth() rejects with AggregateError - one send fails, one succeeds -> auth() resolves (true partial-success path, which the old localhost test never actually exercised) Also ports both tests from the leftover ava API to node:test/assert after the rebase onto main. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: anthropic-code-agent[bot] <242468646+Claude@users.noreply.github.com> Co-authored-by: snichme <102988+snichme@users.noreply.github.com> Co-authored-by: Magnus Landerblom <magnus@landerblom.se> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent a1f2b65 commit 2e288e4

3 files changed

Lines changed: 49 additions & 3 deletions

File tree

src/sparoid.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,13 @@ export async function auth(host: string, port: number, key?: string, hmac_key?:
4747
promises.push(udpSend(hmaced, addr, port));
4848
}
4949
}
50-
await Promise.all(promises)
50+
const results = await Promise.allSettled(promises)
51+
if (results.length === 0 || results.every(r => r.status === 'rejected')) {
52+
throw new AggregateError(
53+
results.map(r => (r as PromiseRejectedResult).reason),
54+
'sparoid auth failed: all knocks failed',
55+
)
56+
}
5157
await sleep(200) // let the server process the packet
5258
}
5359

test/test.ts

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,23 @@
1-
import test from 'node:test'
1+
import test, { mock } from 'node:test'
22
import assert from 'node:assert/strict'
33
import dgram from 'dgram'
44
import * as sparoid from '../src/sparoid.js'
55

66
const key = "0000000000000000000000000000000000000000000000000000000000000000"
77
const loopback = Buffer.from([127, 0, 0, 1])
88

9+
type SendCb = (err?: Error | null) => void
10+
11+
// Replace dgram sockets with a fake whose send() outcome we control, so the
12+
// "all knocks fail" / "partial success" paths don't depend on the host's
13+
// network (IPv6 routing, reachability) — which is what made these tests flaky.
14+
function mockSend(send: (cb: SendCb) => void): void {
15+
mock.method(dgram, 'createSocket', (() => ({
16+
send: (_msg: Buffer, _port: number, _host: string, cb: SendCb) => send(cb),
17+
close: () => {},
18+
})) as unknown as typeof dgram.createSocket)
19+
}
20+
921
test("it can auth", async () => {
1022
const server = dgram.createSocket('udp4')
1123
await new Promise<void>((resolve, reject) => {
@@ -37,3 +49,31 @@ test("raises on error on DNS error", async () => {
3749
await assert.rejects(() => sparoid.auth("none.arpa", 8485, key, key),
3850
{ message: /ENOTFOUND/ })
3951
})
52+
53+
test("throws AggregateError when all knocks fail", async () => {
54+
// Every send fails -> auth() must reject with an aggregated error.
55+
mockSend(cb => cb(new Error('ENETUNREACH')))
56+
try {
57+
await assert.rejects(
58+
() => sparoid.auth("100::1", 8486, key, key, [loopback]),
59+
(err: unknown) =>
60+
err instanceof AggregateError && /all knocks failed/.test(err.message),
61+
)
62+
} finally {
63+
mock.restoreAll()
64+
}
65+
})
66+
67+
test("succeeds when at least one knock succeeds", async () => {
68+
// First knock fails to send, the second succeeds: auth() must still resolve
69+
// (mirrors a dual-stack host where one address family is unreachable).
70+
let calls = 0
71+
mockSend(cb => cb(calls++ === 0 ? new Error('ENETUNREACH') : null))
72+
try {
73+
await assert.doesNotReject(
74+
() => sparoid.auth("127.0.0.1", 8487, key, key, [loopback, loopback]),
75+
)
76+
} finally {
77+
mock.restoreAll()
78+
}
79+
})

tsconfig.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"compilerOptions": {
3-
"target": "ES2020",
3+
"target": "ES2021",
44
"module": "NodeNext",
55
"moduleResolution": "NodeNext",
66
"strict": true,

0 commit comments

Comments
 (0)