A rate limit is a schedule instruction, not a broken feed

packages/db/src/authors.js

@@ -240,7 +240,7 @@ export async function addAuthorLinks(db, authorId, links) {
         link.network,
         link.url,
         link.handle || null,
-        link.source,
+        linkSource(link),
         link.verified ? 1 : 0,
         nowIso(),
       ],
@@ -301,7 +301,7 @@ export async function addFeedLinks(db, feedId, links) {
         link.network,
         link.url,
         link.handle || null,
-        link.source,
+        linkSource(link),
         link.verified ? 1 : 0,
         nowIso(),
       ],
@@ -685,6 +685,31 @@ export async function feedHasAuthors(db, feedId) {
  * @param {Array<object>} [input.feedLinks] accounts to file under the feed
  * @returns {Array<{ sql: string, args: unknown[] }>} in dependency order
  */
+/**
+ * Where a link was found, as a string the database will accept.
+ *
+ * `author_links.source` and `feed_links.source` are both `not null`, and this is
+ * the last place before the wire that can say so. It matters more than a
+ * defensive default usually does, because of *how* the remote client fails:
+ * `undefined` is not a bindable libSQL value, so hrana throws `Unsupported type
+ * of value` while the statement is being serialized -- before any SQL runs, with
+ * no column named and no row to point at. That error surfaced as
+ * `could not be crawled`, which reads like a publisher who is down.
+ *
+ * It cost the directory a day. Every Substack newsletter emits `<itunes:owner>`
+ * and no other byline, so `feedContacts` harvested the mailbox, dropped the
+ * provenance, and every one of those feeds failed its crawl at the write --
+ * 985 of 1,385 crawls in the hour this was found. Local SQLite binds `undefined`
+ * as null without complaint, so the tests and every local run passed.
+ *
+ * @param {{ source?: unknown }} link
+ * @returns {string}
+ */
+function linkSource(link) {
+  const source = link?.source;
+  return typeof source === 'string' && source !== '' ? source : 'feed-document';
+}
+
 export function creditStatements({ feedId, identityKey, slug, person, authorLinks = [], feedLinks = [] }) {
   const now = nowIso();
   const statements = [];
@@ -765,7 +790,7 @@ export function creditStatements({ feedId, identityKey, slug, person, authorLink
         link.network,
         link.url,
         link.handle || null,
-        link.source,
+        linkSource(link),
         link.verified ? 1 : 0,
         now,
         identityKey,
@@ -802,7 +827,7 @@ export function feedLinkStatements(feedId, links) {
         link.network,
         link.url,
         link.handle || null,
-        link.source,
+        linkSource(link),
         link.verified ? 1 : 0,
         nowIso(),
       ],

packages/db/src/queries.js

@@ -2732,6 +2732,31 @@ export async function markCrawlFailure(db, id, error, errorCount, minutes) {
   });
 }
 
+/**
+ * Come back later, and hold every judgement about this feed.
+ *
+ * The counterpart to `markCrawlFailure` for a server that answered 429 -- or 503
+ * with a Retry-After. Only the schedule moves: `status`, `error_count` and
+ * `last_error` are left exactly as they were, and `last_success_at` with them.
+ *
+ * `last_fetched_at` is *not* stamped either, and that is the subtle one. It
+ * means "when we last read this publisher", and a throttle is precisely the case
+ * where we did not read them. Stamping it would make a feed we have been bounced
+ * from for a day look freshly crawled on every page that reports staleness.
+ *
+ * @param {Client} db
+ * @param {string} id
+ * @param {number} minutes
+ * @returns {Promise<void>}
+ */
+export async function markThrottled(db, id, minutes) {
+  const wait = Math.max(1, Math.round(Number(minutes) || 30));
+  await db.execute({
+    sql: 'update feeds set next_fetch_at = ?, updated_at = ? where id = ?',
+    args: [nowIso(wait * 60_000), nowIso(), id],
+  });
+}
+
 /* ------------------------------------------------------------- feed cards */
 
 /**

packages/db/test/link-binds.test.js

@@ -0,0 +1,98 @@
+import assert from 'node:assert/strict';
+import { test } from 'node:test';
+
+import { creditStatements, feedLinkStatements } from '../src/authors.js';
+
+/**
+ * Everything the remote libSQL client will accept as a bound parameter.
+ *
+ * Mirrors `valueToProto` in @libsql/hrana-client: null, string, finite number,
+ * bigint, boolean, ArrayBuffer, Uint8Array, Date and any other object (which is
+ * stringified). What is left -- `undefined`, symbols and functions -- throws
+ * `TypeError: Unsupported type of value` during serialization.
+ *
+ * This is asserted here rather than left to an integration test because the
+ * local SQLite driver used by every test in this repo does *not* share the
+ * restriction: it binds `undefined` as null and passes. The only place the
+ * difference shows up is production.
+ *
+ * @param {unknown} value
+ * @returns {boolean}
+ */
+function bindable(value) {
+  if (value === null) return true;
+  const type = typeof value;
+  if (type === 'undefined' || type === 'symbol' || type === 'function') return false;
+  if (type === 'number') return Number.isFinite(value);
+  return true;
+}
+
+/**
+ * @param {Array<{ sql: string, args: unknown[] }>} statements
+ */
+function assertBindable(statements) {
+  for (const statement of statements) {
+    for (const [index, arg] of statement.args.entries()) {
+      assert.ok(
+        bindable(arg),
+        `arg ${index} (${String(arg)}) cannot be bound: ${statement.sql.slice(0, 80)}`,
+      );
+    }
+  }
+}
+
+const person = {
+  name: 'Marta Nowak',
+  normName: 'marta nowak',
+  bio: '',
+  avatarUrl: '',
+  siteUrl: '',
+  email: 'marta@example.com',
+  confidence: 0.85,
+  role: 'owner',
+  evidence: 'itunes-owner',
+};
+
+test('a link that names no source is still storable', () => {
+  // The crawler-stopping bug, at the layer that has to be right whatever the
+  // callers do. `feed_links.source` and `author_links.source` are `not null`,
+  // and a link arriving without one used to bind `undefined` -- which the
+  // remote client refuses outright, failing the entire crawl transaction that
+  // carried the feed row and its posts. Every Substack newsletter in the
+  // directory produced exactly this shape.
+  const link = { network: 'email', url: 'mailto:marta@example.com' };
+
+  const statements = creditStatements({
+    feedId: 'feed-1',
+    identityKey: 'marta@example.com',
+    slug: 'marta-nowak',
+    person,
+    authorLinks: [link],
+    feedLinks: [link],
+  });
+
+  assertBindable(statements);
+  assertBindable(feedLinkStatements('feed-1', [link]));
+});
+
+test('a link that does name its source keeps it', () => {
+  const link = { network: 'email', url: 'mailto:marta@example.com', source: 'rel-me' };
+  const [statement] = feedLinkStatements('feed-1', [link]);
+
+  assert.ok(statement.args.includes('rel-me'));
+  assertBindable([statement]);
+});
+
+test('a whole credit binds cleanly when the person is bare', () => {
+  // A credit carrying nothing but a name -- no bio, no avatar, no site, no
+  // email -- is the common case on the small web, and every one of those holes
+  // is a bound parameter.
+  const statements = creditStatements({
+    feedId: 'feed-1',
+    identityKey: 'someone@example.com',
+    slug: 'someone',
+    person: { name: 'Someone', confidence: 0.4, role: 'author' },
+  });
+
+  assertBindable(statements);
+});

packages/feed/src/fetch.js

@@ -104,7 +104,7 @@ export async function isPublicHost(hostname) {
  * @param {string} url
  * @param {{ etag?: string|null, lastModified?: string|null,
  *   headers?: Record<string, string> }} [conditional]
- * @returns {Promise<{ ok: boolean, status: number, contentType: string, body: string, url: string, notModified?: boolean, etag?: string|null, lastModified?: string|null, error?: string }>}
+ * @returns {Promise<{ ok: boolean, status: number, contentType: string, body: string, url: string, notModified?: boolean, etag?: string|null, lastModified?: string|null, retryAfter?: number|null, error?: string }>}
  */
 export async function safeFetch(url, conditional = {}) {
   const normalized = normalizeUrl(url);
@@ -196,6 +196,7 @@ export async function safeFetch(url, conditional = {}) {
       contentType: res.headers.get('content-type') ?? '',
       body,
       url: res.url,
+      retryAfter: retryAfterSeconds(res.headers.get('retry-after')),
       etag,
       lastModified,
     };
@@ -378,6 +379,31 @@ function concat(chunks, size) {
   return out;
 }
 
+/**
+ * How long a server asked us to wait, in seconds.
+ *
+ * RFC 9110 allows either a delay in seconds or an HTTP date, and both are seen
+ * in the wild. Anything unparseable is null, which the caller reads as "throttled
+ * but unsaid" and answers with its own default rather than with zero.
+ *
+ * Clamped to a day. A server that asks for a month has almost certainly sent us
+ * a date we misread, and honouring it literally would retire the feed.
+ *
+ * @param {string|null} header
+ * @returns {number|null}
+ */
+export function retryAfterSeconds(header) {
+  if (!header) return null;
+
+  const seconds = Number(String(header).trim());
+  if (Number.isFinite(seconds) && seconds >= 0) return Math.min(Math.round(seconds), 86_400);
+
+  const at = Date.parse(String(header));
+  if (!Number.isNaN(at)) return Math.min(Math.max(0, Math.round((at - Date.now()) / 1000)), 86_400);
+
+  return null;
+}
+
 /**
  * Resolve whatever a user submitted into a parsed feed.
  *
@@ -409,7 +435,19 @@ export async function resolveFeed(input, conditional = {}) {
   if (first.notModified) {
     return { ok: false, notModified: true, etag: first.etag, lastModified: first.lastModified };
   }
-  if (!first.ok) return { ok: false, error: first.error ?? `http-${first.status}` };
+  // A throttle is not a broken feed, and the difference has to survive this
+  // return or the crawler cannot tell them apart. 429 is the explicit form; 503
+  // with a Retry-After is the same statement from a server that is briefly
+  // unwilling rather than permanently unable. Both mean "come back later", which
+  // is a schedule instruction, not evidence about the publisher.
+  if (!first.ok) {
+    const throttled = first.status === 429 || (first.status === 503 && first.retryAfter != null);
+    return {
+      ok: false,
+      error: first.error ?? `http-${first.status}`,
+      ...(throttled ? { throttled: true, retryAfter: first.retryAfter ?? null } : {}),
+    };
+  }
 
   if (looksLikeFeed(first.contentType, first.body, first.url)) {
     const feed = parseFeed(first.body, first.url);

packages/feed/src/identity.js

@@ -749,10 +749,16 @@ export function channelCreditInputs(channel, format, base = '') {
  * routed it there -- collecting it again here would file one address in two
  * places and count it twice.
  *
+ * Each contact carries the channel element it was read from, the same way a
+ * credit does. `feed_links.source` is `not null` and is what the page means by
+ * "where we found this" -- a mailbox harvested from `webMaster` is a weaker
+ * claim than one from `itunes:owner`, and dropping the provenance here lost
+ * that distinction *and* left the column with nothing to store.
+ *
  * @param {any} channel the raw parsed channel/feed object
  * @param {'rss'|'atom'|'rdf'|'json'} format
  * @param {string} [base]
- * @returns {Array<{ url: string, network: string }>}
+ * @returns {Array<{ url: string, network: string, source: string }>}
  */
 export function feedContacts(channel, format, base = '') {
   const out = [];
@@ -776,7 +782,7 @@ export function feedContacts(channel, format, base = '') {
       const link = candidate ? classifyLink(candidate, base) : null;
       if (!link || seen.has(link.url)) continue;
       seen.add(link.url);
-      out.push({ url: link.url, network: link.network });
+      out.push({ url: link.url, network: link.network, source: input.source });
     }
   }
 

packages/feed/src/parse.js

@@ -118,6 +118,31 @@ function hasVideoEnclosure(item) {
   );
 }
 
+/**
+ * Does this element carry audio?
+ *
+ * @param {any} item
+ * @returns {boolean}
+ */
+function hasAudioEnclosure(item) {
+  if (arr(item?.enclosure).some((e) => AUDIO_TYPE.test(String(e?.['@type'] ?? '')))) return true;
+  return arr(item?.link).some(
+    (l) => l?.['@rel'] === 'enclosure' && AUDIO_TYPE.test(String(l?.['@type'] ?? '')),
+  );
+}
+
+/**
+ * Tags no platform sets unless it is publishing a show.
+ *
+ * The rest of `PODCAST_CHANNEL_TAGS` is weaker than it looks. Substack emits
+ * `<itunes:owner>` on every publication it hosts, podcast or not, so on its own
+ * that tag files a text newsletter under podcasts -- and Substack is thousands
+ * of feeds here. `itunes:type` and the `podcast:` namespace are different: they
+ * are written by podcast hosting, for podcast directories, and nothing else has
+ * a reason to emit them.
+ */
+const PODCAST_DECLARED_TAGS = ['itunes:type', 'podcast:guid', 'podcast:medium'];
+
 /**
  * How much prose an item carries of its own, in characters of text.
  *
@@ -406,8 +431,9 @@ function isNewsroom(channel, items) {
  * guessing. Then YouTube, because a channel feed says so in its own namespace
  * and nothing else needs weighing. Then video, which is an enclosure *and*
  * corroboration that the enclosure is the point. Then podcast, which is a
- * publisher who filled in the podcast namespaces. Everything else is a blog,
- * which is what the overwhelming majority of the directory is.
+ * publisher who filled in the podcast namespaces *and* ships audio, or who
+ * declared a show outright. Everything else is a blog, which is what the
+ * overwhelming majority of the directory is.
  *
  * One correction, arrived at twice from opposite ends of the directory: an
  * attachment is not a genre. A post with a file on it is still a post, and
@@ -455,7 +481,23 @@ function kindOfChannel(channel, items) {
   const withVideo = sample.filter(hasVideoEnclosure);
   if (withVideo.length > 0 && (podcastTags || isShowShaped(sample, withVideo))) return KIND_VIDEO;
 
-  if (podcastTags) return KIND_PODCAST;
+  // A podcast publishes audio, and the same correction applies here as to video
+  // above: the tag has to be corroborated. A show attaches an episode to every
+  // entry, so one audio enclosure anywhere in the sample is enough -- and a feed
+  // with podcast tags and not a single audio file in five items is a newsletter
+  // whose host fills in the iTunes block. Substack does that for every
+  // publication it serves, which put thousands of text newsletters under
+  // /podcasts.
+  //
+  // A declaration still stands on its own: `itunes:type` and the `podcast:`
+  // namespace are the publisher stating what they made, and a show that has not
+  // released its first episode yet is still a podcast. So is a feed we have no
+  // items for -- with nothing sampled there is no evidence to corroborate, and
+  // the tags are the only thing said.
+  const declaredPodcast = PODCAST_DECLARED_TAGS.some((tag) => channel?.[tag] !== undefined);
+  if (podcastTags && (declaredPodcast || sample.length === 0 || sample.some(hasAudioEnclosure))) {
+    return KIND_PODCAST;
+  }
 
   // No audio branch, deliberately. Audio without a declared medium is a blog
   // that narrated itself, and `declaredMedium` above is the only way to music.