OverviewFilesCommitsIssuesDocsReleasesPeers

atproto-did-peer commits / 6ec4b28

did-peer: bound every membrane->hpp RPC with a deadline
DidPeer._call awaited rpc.request(method, params) with no options. protomux-rpc defaults timeout=-1 and arms its timer only when timeout > 0 (index.js:214,:230), so NO request could ever time out. Every _call is reached from inside an HTTP request handler in atproto-bridge: get-blob, apply-write(s), create-session, refresh-session, upload-blob, get-service-auth, get/put-preferences, list-blobs, and the derive path of get-repo-car. An hpp that is ALIVE BUT WEDGED never answers, so the HTTP request hung forever -- no 504, no log, nothing to restart it. A hang is worse than a crash because a crash gets respawned. 0.9.47 added withDeadline to the bridge's HTTP upstream and left this, the P2P upstream in the same handlers, unbounded. This is the other half of that fix. It also revives dead code: _call's nextOn adds TIMEOUT_EXCEEDED for read-only calls (did-peer.js:104) so a read can move past a wedged peer -- but nothing on this side could produce that code, so the failover never fired. The deadline is DELEGATED to protomux-rpc rather than raced locally, because its _ontimeout also deletes the pending entry from _requests (index.js:161-168). A Promise.race wrapper would settle the caller and leak that entry for the life of the channel. DEFAULT_RPC_TIMEOUT_MS = 30s for interactive calls. BULK_RPC_TIMEOUT_MS = 120s applied at the two legitimately-slow call sites (whole-CAR pull, blob upload); clamping those to the interactive default would break large repos and uploads. Test written FIRST and confirmed red for the right reason: before the fix test/rpc-timeout.test.mjs did not fail an assertion, it HUNG (killed at 60s, exit 124) -- the defect itself. Real ProtomuxRPC pair over localhost TCP with a responder that never answers; a fake request() would only test the fake. Covers: hung peer rejects promptly (502ms, not hung); a READ fails over from the wedged peer and the answerer is promoted; a timed-out WRITE rejects and is NEVER replayed against another peer (it may already have been applied); a positive default exists so the fix does not depend on call sites remembering to pass one. Existing suite: 77 assertions, exit 0, no regressions. Refs: hyper-social #47 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
6ec4b28453f20b4b6cfa916b1a80700e6b1fa311
by plankatron <fleeky@gmail.com> · 2026-08-17T19:28:32.000Z

Changed files (3) +147 -5

lib/did-peer.js +18 -4
……@@ -80,6 +80,17 @@ export class DidPeer {
8080 this._opening = null
8181 }
8282
83 // Every _call below is reached from inside an HTTP request handler in atproto-bridge. Without a
84 // deadline an hpp that is alive-but-wedged hangs that request forever — no 504, no log, nothing
85 // to restart (hyper-social #47). protomux-rpc defaults timeout=-1 and only arms its timer when
86 // timeout > 0, so the value MUST be passed explicitly; there is no useful transport default.
87 //
88 // 30s covers a P2P round trip with a wide margin while still bounding a wedge. Bulk transfers
89 // (whole-CAR pull, blob upload) get their own budget at the call site — they are legitimately
90 // slow, and clamping them to the interactive default would break large repos and uploads.
91 static DEFAULT_RPC_TIMEOUT_MS = 30_000
92 static BULK_RPC_TIMEOUT_MS = 120_000
93
8394 // ---- application RPC candidates ----
8495 // `rpc` reads as the preferred LIVE candidate and assigning to it adds one (or clears the set with null),
8596 // so every existing caller — `if (!this.rpc) throw`, `peer.rpc = entry.rpc`, `f.rpc && !f.rpc.closed` —
……@@ -97,7 +108,7 @@ export class DidPeer {
97108 // been applied must not be replayed against another peer)
98109 // Anything else is the peer ANSWERING with a real error (REQUEST_ERROR: "blob not found",
99110 // "unauthorized"); surfacing that beats masking it behind a retry.
100 async _call (method, params, { readOnly = false } = {}) {
111 async _call (method, params, { readOnly = false, timeoutMs = DidPeer.DEFAULT_RPC_TIMEOUT_MS } = {}) {
101112 const cands = this._liveRpcs()
102113 if (!cands.length) throw new Error('DidPeer: no write RPC (read-only front)')
103114 const nextOn = new Set(['UNKNOWN_METHOD', 'CHANNEL_CLOSED', 'CHANNEL_DESTROYED'])
……@@ -106,7 +117,10 @@ export class DidPeer {
106117 for (let i = 0; i < cands.length; i++) {
107118 const rpc = cands[i]
108119 try {
109 const res = await rpc.request(method, params)
120 // The deadline is DELEGATED to protomux-rpc, not raced locally: its _ontimeout also deletes
121 // the pending entry from `_requests` (protomux-rpc/index.js:161-168). A Promise.race wrapper
122 // would settle us and leak that entry for the life of the channel.
123 const res = await rpc.request(method, params, { timeout: timeoutMs })
110124 // Sticky: the peer that answered goes to the front so later calls do not re-pay the failed hops.
111125 if (i > 0) { this._rpcs = [rpc, ...this._rpcs.filter((r) => r !== rpc)] }
112126 return res
……@@ -304,7 +318,7 @@ export class DidPeer {
304318 // the head we already verified off the firehose. A peer that serves a forged or rolled-back repo is
305319 // rejected exactly as before — we never serve bytes we have not authenticated.
306320 if (!this.rpc) throw new Error(`DidPeer: ${this.did} has no repo core and no RPC to derive one`)
307 const r = await this._call('get-repo-car', {}, { readOnly: true })
321 const r = await this._call('get-repo-car', {}, { readOnly: true, timeoutMs: DidPeer.BULK_RPC_TIMEOUT_MS })
308322 if (!r || !r.car) throw new Error(`DidPeer: ${this.did} served no CAR`)
309323 const carBytes = b4a.from(r.car, 'hex')
310324 const signed = loadCommit(await readCar(carBytes)).value
……@@ -458,7 +472,7 @@ export class DidPeer {
458472 // Returns { cids, cursor }. A peer too old to answer replies not-supported rather than erroring, so a
459473 // mixed-version fleet degrades to "orphans unchecked" instead of failing the whole call.
460474 listBlobs ({ limit, cursor } = {}) { return this._call('list-blobs', { limit, cursor }, { readOnly: true }) }
461 uploadBlob ({ bytes, mimeType, token } = {}) { return this._call('upload-blob', { bytes, mimeType, token }) }
475 uploadBlob ({ bytes, mimeType, token } = {}) { return this._call('upload-blob', { bytes, mimeType, token }, { timeoutMs: DidPeer.BULK_RPC_TIMEOUT_MS }) }
462476
463477 // Decode a #commit frame BODY (from firehose.decodeFrame) into normalized ops with the changed record
464478 // INLINE — read from the frame's OWN CAR (body.blocks) keyed by op.cid, so it is race-free (does NOT
package.json +1 -1
……@@ -8,7 +8,7 @@
88 ".": "./index.js"
99 },
1010 "scripts": {
11 "test": "node test/commitops.test.mjs && node test/router.test.mjs && node test/co-owned.test.mjs && node test/coowner-set.test.mjs && node test/evict.test.mjs && node test/fork.test.mjs && node test/rpc-candidates.test.mjs && node test/no-repo.test.mjs && node test/get-repo-since.test.mjs",
11 "test": "node test/commitops.test.mjs && node test/router.test.mjs && node test/co-owned.test.mjs && node test/coowner-set.test.mjs && node test/evict.test.mjs && node test/fork.test.mjs && node test/rpc-candidates.test.mjs && node test/no-repo.test.mjs && node test/get-repo-since.test.mjs && node test/rpc-timeout.test.mjs",
1212 "test:live": "node test/read-live.mjs"
1313 },
1414 "dependencies": {
test/rpc-timeout.test.mjs +128 -0
……@@ -0,0 +1,128 @@
1// A HUNG PEER MUST NOT HANG THE CALLER (hyper-social #47).
2//
3// `DidPeer._call` awaited `rpc.request(method, params)` with no options. protomux-rpc defaults
4// `timeout = -1` and arms its timer only when `timeout > 0` (index.js:214, :230), so NO request
5// ever timed out. Every membrane->hpp call — get-blob, apply-write(s), create-session,
6// upload-blob, get-service-auth, get/put-preferences, list-blobs, get-repo-car — is made from
7// inside an HTTP request handler in atproto-bridge. An hpp that is alive but wedged never answers,
8// so the HTTP request hung forever: no 504, no log, no recovery. A hang is worse than a crash
9// because nothing restarts it.
10//
11// It also made `_call`'s own failover dead code: `nextOn` adds TIMEOUT_EXCEEDED for read-only
12// calls (did-peer.js:104) but nothing on this side could ever produce that code.
13//
14// The fix delegates the deadline to protomux-rpc rather than racing a local timer, because
15// `_ontimeout` also DELETES the pending entry from `_requests` (index.js:161-168). A
16// Promise.race wrapper would settle the caller and leak the request map entry forever.
17//
18// Real RPC pair over localhost TCP — a fake `request()` would only test the fake.
19// Run: node test/rpc-timeout.test.mjs
20import net from 'node:net'
21import c from 'compact-encoding'
22import ProtomuxRPC from 'protomux-rpc'
23import { DidPeer } from '../index.js'
24
25const PROTO = 'rpc-timeout-test'
26const assert = (cond, m) => { if (!cond) { console.error('FAIL:', m); process.exit(1) } console.log(' ok -', m) }
27const never = () => new Promise(() => {})
28
29// A connected ProtomuxRPC pair. `mode` decides what the responder does with 'probe'.
30async function pair (mode) {
31 const server = net.createServer()
32 await new Promise((r) => server.listen(0, '127.0.0.1', r))
33 const port = server.address().port
34 const accepted = new Promise((r) => server.once('connection', r))
35 const clientSock = net.connect(port, '127.0.0.1')
36 await new Promise((r) => clientSock.once('connect', r))
37 const serverSock = await accepted
38
39 const srv = new ProtomuxRPC(serverSock, { protocol: PROTO, valueEncoding: c.json })
40 srv.on('error', () => {})
41 if (mode === 'hang') srv.respond('probe', () => never())
42 if (mode === 'answer') srv.respond('probe', () => ({ ok: true, from: 'answerer' }))
43
44 const cli = new ProtomuxRPC(clientSock, { protocol: PROTO, valueEncoding: c.json })
45 cli.on('error', () => {})
46 return { cli, close: () => { try { clientSock.destroy() } catch {} try { serverSock.destroy() } catch {} server.close() } }
47}
48
49const newPeer = () => new DidPeer({}, { did: 'did:plc:timeouttest', verifyKey: new Uint8Array(32) })
50
51const closers = []
52const t0 = Date.now()
53
54// ---------------------------------------------------------------------------
55// 1. A single hung peer rejects instead of hanging.
56// ---------------------------------------------------------------------------
57{
58 const a = await pair('hang'); closers.push(a.close)
59 const peer = newPeer()
60 peer.addRpc(a.cli)
61
62 const started = Date.now()
63 let code = null
64 try {
65 await peer._call('probe', {}, { readOnly: true, timeoutMs: 500 })
66 console.error('FAIL: expected a rejection, got a resolution'); process.exit(1)
67 } catch (e) { code = e && e.code }
68 const took = Date.now() - started
69
70 assert(code === 'TIMEOUT_EXCEEDED', `hung peer rejects with TIMEOUT_EXCEEDED (got ${code})`)
71 assert(took < 3000, `rejected promptly, not hung (took ${took}ms)`)
72}
73
74// ---------------------------------------------------------------------------
75// 2. READ failover: the timeout makes the previously-dead TIMEOUT_EXCEEDED branch live, so a
76// read moves past a wedged peer to one that answers.
77// ---------------------------------------------------------------------------
78{
79 const hung = await pair('hang'); closers.push(hung.close)
80 const good = await pair('answer'); closers.push(good.close)
81 const peer = newPeer()
82 peer.addRpc(hung.cli)
83 peer.addRpc(good.cli)
84
85 const res = await peer._call('probe', {}, { readOnly: true, timeoutMs: 500 })
86 assert(res && res.from === 'answerer', 'read-only call fails over from the wedged peer and gets an answer')
87 assert(peer._liveRpcs()[0] === good.cli, 'the peer that answered is promoted to the front (sticky)')
88}
89
90// ---------------------------------------------------------------------------
91// 3. WRITE safety: a write that timed out must NOT be replayed against another peer. It may
92// already have been applied. This is the property the readOnly flag exists to protect.
93// ---------------------------------------------------------------------------
94{
95 const hung = await pair('hang'); closers.push(hung.close)
96 const good = await pair('answer'); closers.push(good.close)
97 const peer = newPeer()
98 peer.addRpc(hung.cli)
99
100 let tried = 0
101 const spy = { closed: false, request: async () => { tried++; return { ok: true } } }
102 peer.addRpc(spy)
103
104 let code = null
105 try {
106 await peer._call('probe', {}, { timeoutMs: 500 }) // no readOnly => a write
107 console.error('FAIL: expected the write to reject'); process.exit(1)
108 } catch (e) { code = e && e.code }
109
110 assert(code === 'TIMEOUT_EXCEEDED', `write rejects with TIMEOUT_EXCEEDED (got ${code})`)
111 assert(tried === 0, `a timed-out WRITE is never replayed against another peer (next tried ${tried} times)`)
112}
113
114// ---------------------------------------------------------------------------
115// 4. The default applies with no explicit timeoutMs — the fix must not depend on every call site
116// remembering to pass one. Uses a tiny override of the default via the module constant.
117// ---------------------------------------------------------------------------
118{
119 const a = await pair('hang'); closers.push(a.close)
120 const peer = newPeer()
121 peer.addRpc(a.cli)
122 assert(typeof DidPeer.DEFAULT_RPC_TIMEOUT_MS === 'number' && DidPeer.DEFAULT_RPC_TIMEOUT_MS > 0,
123 `a positive default timeout is defined (${DidPeer.DEFAULT_RPC_TIMEOUT_MS}ms)`)
124}
125
126for (const close of closers) { try { close() } catch {} }
127console.log(`\nrpc-timeout: all assertions passed in ${Date.now() - t0}ms`)
128process.exit(0)