Skip to content

Commit 3683486

Browse files
ralyodioclaude
andauthored
fix(x): only a missing account may count against a source's health (#157)
`fetchXSource` treated every failure except a rate limit as evidence about the account. So a provider outage, a dead session, or no provider being configured at all incremented `error_count` — and `markCrawlFailure` retires a feed at ten consecutive failures. An X source polls on a five-minute floor. Ten strikes is fifty minutes. That makes the ordinary order of operations destructive: setting `X_ENABLED=true` before a provider is reachable is how you find out whether it is reachable, and it would have marked every X source dead within the hour, logging "these accounts are broken" rather than "nothing is configured to collect with". The comment above the line already claimed the correct behaviour — that a deleted, suspended or protected account is the one failure genuinely about the source. The code did the opposite. Now only `XNoSuchSource` reaches `markCrawlFailure`; everything else returns `throttled`, which moves `next_fetch_at` and leaves `status`, `error_count`, `last_error` and `last_success_at` untouched. Retry intervals by cause, because they are three different guesses about when the situation changes: a rate limit uses the interval the server named; an outage waits twenty minutes; "no provider configured" waits an hour, since nothing changes until somebody deploys something. Five tests, four of which fail against the merged code. Claude-Session: https://claude.ai/code/session_01Q6QEgpuS4MLamogXtr2ZX6 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 4ab9ca9 commit 3683486

2 files changed

Lines changed: 122 additions & 9 deletions

File tree

‎packages/social/src/x/fetch.js‎

Lines changed: 51 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -109,16 +109,34 @@ export async function fetchXSource(feed, opts) {
109109
{ sessions, onEvent, signal: opts.signal },
110110
);
111111
} catch (error) {
112-
// A rate limit is a schedule instruction, not evidence about the account.
113-
// Returning it as `throttled` routes it to `markThrottled`, which moves
114-
// `next_fetch_at` and leaves every health column alone — the same treatment
115-
// an ordinary publisher's 429 gets, and for the same reason (§16).
116-
if (error?.name === 'XRateLimited') {
117-
return { ok: false, throttled: true, retryAfter: error.retryAfter ?? null, error: 'rate-limited' };
112+
// **Exactly one kind of failure is about the source.** A deleted, suspended
113+
// or protected account is a fact about the account; everything else — a rate
114+
// limit, a provider outage, a dead session, no provider configured at all —
115+
// is a fact about us or about the upstream, and recording it against the
116+
// account is how a directory deletes itself.
117+
//
118+
// The arithmetic, because it is what makes this urgent rather than tidy:
119+
// `markCrawlFailure` retires a feed at ten consecutive failures, and an X
120+
// source polls on a five-minute floor. Ten strikes is **fifty minutes**. So
121+
// switching X on before a provider is reachable — which is the ordinary
122+
// order of operations, since the flag is how you find out — would quietly
123+
// mark every X source dead within the hour, and nothing in the logs would
124+
// say "no provider configured" rather than "these accounts are broken".
125+
//
126+
// Returning `throttled` routes to `markThrottled`, which moves
127+
// `next_fetch_at` and leaves `status`, `error_count`, `last_error` and
128+
// `last_success_at` exactly as they were — the same treatment an ordinary
129+
// publisher's 429 gets, and for the same reason (§16, §40).
130+
if (error?.name === 'XNoSuchSource') {
131+
return { ok: false, error: String(error?.message ?? 'no-such-source').slice(0, 200) };
118132
}
119-
// A deleted, suspended or protected account is the one failure that is
120-
// genuinely about the source, so it is the one that counts against it.
121-
return { ok: false, error: String(error?.message ?? 'x-fetch-failed').slice(0, 200) };
133+
134+
return {
135+
ok: false,
136+
throttled: true,
137+
retryAfter: retryAfterFor(error),
138+
error: String(error?.message ?? 'x-fetch-failed').slice(0, 200),
139+
};
122140
}
123141

124142
const posts = result.posts ?? [];
@@ -149,6 +167,30 @@ export async function fetchXSource(feed, opts) {
149167
};
150168
}
151169

170+
/**
171+
* How long to wait before trying this source again, by what went wrong.
172+
*
173+
* The three intervals are three different guesses about when the situation
174+
* changes. A rate limit usually names its own; a provider outage is minutes;
175+
* and "no provider is configured" is a deployment that has not happened yet, so
176+
* asking again in ten minutes is a thousand pointless wake-ups a day across the
177+
* directory and answers no sooner than asking in an hour.
178+
*
179+
* @param {Error & { retryAfter?: number|null }} error
180+
* @returns {number} seconds
181+
*/
182+
function retryAfterFor(error) {
183+
if (error?.name === 'XRateLimited') {
184+
return Number(error.retryAfter) > 0 ? Number(error.retryAfter) : ANOMALY_MINUTES * 60;
185+
}
186+
// Nothing is set up to collect with. Distinguished by message rather than by
187+
// type because `XUnavailable` covers both this and a provider that is merely
188+
// down, and the two deserve very different patience.
189+
if (/no X provider is configured/i.test(String(error?.message ?? ''))) return 3600;
190+
191+
return ANOMALY_MINUTES * 60;
192+
}
193+
152194
/**
153195
* The per-source toggles of §6.3, with the PRD's defaults.
154196
*

‎packages/social/test/providers.test.js‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,3 +445,74 @@ test('a source whose ref we cannot read fails loudly rather than crawling nothin
445445
assert.equal(result.ok, false);
446446
assert.equal(result.error, 'invalid-x-ref');
447447
});
448+
449+
/* --------------------------------------------- what a failure may cost a source */
450+
451+
/*
452+
* `markCrawlFailure` retires a feed at ten consecutive failures and an X source
453+
* polls on a five-minute floor, so ten strikes is fifty minutes. That arithmetic
454+
* is why exactly one of the four errors may reach it.
455+
*/
456+
457+
/** @param {Error} error */
458+
async function failWith(error) {
459+
const registry = new XRegistry({
460+
env: { X_PRIMARY_PROVIDER: 'rsshub', X_FALLBACK_PROVIDERS: '' },
461+
providers: { rsshub: fake('rsshub', [error]) },
462+
});
463+
464+
return fetchXSource(
465+
{ social_ref: 'x:user:openai', feed_url: 'https://x.com/OpenAI', item_count: 5 },
466+
{ runtime: { registry, sessions: null, onEvent: () => {} } },
467+
);
468+
}
469+
470+
test('only a missing account counts against the source', async () => {
471+
const gone = await failWith(new XNoSuchSource('no such account'));
472+
assert.equal(gone.ok, false);
473+
assert.equal(gone.throttled, undefined, 'a deleted account is a fact about the account');
474+
});
475+
476+
test('a provider outage reschedules rather than blaming the account', async () => {
477+
const down = await failWith(new XUnavailable('rsshub: upstream-502'));
478+
assert.equal(down.ok, false);
479+
assert.equal(down.throttled, true);
480+
assert.ok(down.retryAfter > 0);
481+
});
482+
483+
test('a dead session reschedules — it is our credential, not their account', async () => {
484+
const dead = await failWith(new XAuthFailed('auth-failed-401'));
485+
assert.equal(dead.throttled, true);
486+
});
487+
488+
test('X switched on before a provider exists must not retire the directory', async () => {
489+
// The ordinary order of operations: the flag is how you find out whether the
490+
// provider is reachable. With no provider configured the registry refuses
491+
// outright, and that refusal must never look like a broken account.
492+
const registry = new XRegistry({
493+
env: { X_PRIMARY_PROVIDER: 'rsshub', X_FALLBACK_PROVIDERS: '' },
494+
providers: {
495+
rsshub: { ...fake('rsshub', [[]]), configured: () => false },
496+
},
497+
});
498+
499+
const result = await fetchXSource(
500+
{ social_ref: 'x:user:openai', feed_url: 'https://x.com/OpenAI', item_count: 5 },
501+
{ runtime: { registry, sessions: null, onEvent: () => {} } },
502+
);
503+
504+
assert.equal(result.ok, false);
505+
assert.equal(result.throttled, true, 'no provider configured is a deployment state, not a dead account');
506+
// An hour, not ten minutes: nothing changes until somebody deploys something.
507+
assert.equal(result.retryAfter, 3600);
508+
});
509+
510+
test('a rate limit still carries the interval the server named', async () => {
511+
const limited = await failWith(new XRateLimited('429', { retryAfter: 90 }));
512+
assert.equal(limited.throttled, true);
513+
assert.equal(limited.retryAfter, 90);
514+
515+
// And falls back to something sane when it named none.
516+
const bare = await failWith(new XRateLimited('429'));
517+
assert.ok(bare.retryAfter > 0);
518+
});

0 commit comments

Comments
 (0)