Addressed freezing indefinitely when requests are aborted.

config/default.js

@@ -60,6 +60,13 @@ module.exports = {
         avoidDuplicates: true,
         retries: 3,
         concurrency: 10,
+        // wall-clock ceiling (ms) for a single item-download attempt (src/fetch/item.js).
+        // bhttp has no timeout of its own - a request that connects but then just stops
+        // responding (a stalled/misbehaving proxy tunnel is a common cause) would otherwise
+        // hang forever, and since the limiter's concurrency is capped, that one stuck request
+        // blocks every other download queued behind it indefinitely. This bounds each attempt
+        // so a stall fails (and retries, per `retries` above) instead of hanging forever.
+        timeout: 60000,
         watch: {
             schedule: '*/30 * * * *',
         },
@@ -102,6 +109,13 @@ module.exports = {
     proxy: {
         use: true,
         default: null,
+        // if a request fails (including timing out - see fetch.timeout above) and is about to
+        // be retried, retry it using whichever of proxied/direct is NOT what would normally
+        // apply for that host - e.g. the proxy itself is down or being blocked by the target
+        // site, or (the reverse) a host reachable directly is refusing unproxied traffic.
+        // Off by default: for a privacy-sensitive proxy (e.g. Tor) silently falling back to a
+        // direct connection on failure is very much not what you want.
+        retryFlipped: false,
         hosts: {
             // hostnames are matched by suffix, so e.g. 'reddit.com' also covers
             // 'www.reddit.com', but not 'v.redd.it' or 'redd.it' - those need their own

src/fetch/item.js

@@ -10,6 +10,21 @@ const { agentFor } = require('../proxy');
 const logger = require('../logger')(__filename);
 const limiter = require('../limiter').items;
 
+// bhttp has no timeout at all by default - if the server (or, more likely, a proxy tunnel
+// that connects fine but then never relays anything) just stops responding mid-request without
+// erroring, the returned promise never settles. With the limiter's concurrency capped, that one
+// stuck request blocks every other download queued behind it, indefinitely. Race it against a
+// hard wall-clock ceiling so a stall fails (and retries, below) instead of hanging forever.
+function withTimeout(promise, ms, url) {
+    let timer;
+
+    const timedOut = new Promise((resolve, reject) => {
+        timer = setTimeout(() => reject(new Error(`Timed out after ${ms}ms fetching '${url}'`)), ms);
+    });
+
+    return Promise.race([promise, timedOut]).finally(() => clearTimeout(timer));
+}
+
 async function fetchItem(url, attempt, context) {
     async function retry(error) {
         logger.warn(`Failed to fetch '${url}', ${attempt < config.fetch.retries ? 'retrying' : 'giving up'}: ${error.message} (${context.post ? context.post.permalink : 'no post'})`);
@@ -23,7 +38,15 @@ async function fetchItem(url, attempt, context) {
 
     try {
         // throw new Error('Failed it!');
-        const res = await limiter.schedule(async () => bhttp.get(url, { headers: context.headers, agent: agentFor(url) }));
+        // config.proxy.retryFlipped: once this url has already failed once, retry it via
+        // whichever of proxied/direct is NOT what it'd normally get - see resolveProxyUrl()
+        const flip = config.proxy.retryFlipped && attempt > 0;
+
+        const res = await limiter.schedule(async () => withTimeout(
+            bhttp.get(url, { headers: context.headers, agent: agentFor(url, { flip }) }),
+            config.fetch.timeout,
+            url,
+        ));
 
         if (res.statusCode !== 200) {
             throw new Error(`Response not OK for ${url} (${res.statusCode})`);

src/proxy.js

@@ -21,7 +21,7 @@ function hostMatches(hostname, pattern) {
     return hostname === normalized || hostname.endsWith(`.${normalized}`);
 }
 
-function resolveProxyUrl(hostname) {
+function resolveProxyUrl(hostname, { flip = false } = {}) {
     if (config.proxy?.use === false) {
         return null;
     }
@@ -29,19 +29,32 @@ function resolveProxyUrl(hostname) {
     const hosts = config.proxy?.hosts || {};
     const matchedPattern = Object.keys(hosts).find((pattern) => hostMatches(hostname, pattern));
 
-    if (!matchedPattern) {
+    const normal = (() => {
-        return config.proxy?.default || null;
+        if (!matchedPattern) {
+            return config.proxy?.default || null;
+        }
+
+        const matched = hosts[matchedPattern];
+
+        // `true` explicitly opts this host into `default`, `null`/`false`/'' explicitly opts it
+        // out (rather than falling through to `default`), anything else is a per-host override
+        if (matched === true) {
+            return config.proxy?.default || null;
+        }
+
+        return matched || null;
+    })();
+
+    if (!flip) {
+        return normal;
     }
 
-    const matched = hosts[matchedPattern];
+    // used for config.proxy.retryFlipped: retry via whichever of proxied/direct is NOT what
-
+    // this host would normally get. A request that would normally be proxied (whether via
-    // `true` explicitly opts this host into `default`, `null`/`false`/'' explicitly opts it
+    // `default` or a per-host override) retries direct; a request that would normally go
-    // out (rather than falling through to `default`), anything else is a per-host override
+    // direct (no proxy configured for this host, or explicitly excluded from `default`)
-    if (matched === true) {
+    // retries through `default`, if one is set.
-        return config.proxy?.default || null;
+    return normal ? null : (config.proxy?.default || null);
-    }
-
-    return matched || null;
 }
 
 // 'socks4a'/'socks5h' (as opposed to plain 'socks4'/'socks5') tell socks-proxy-agent to
@@ -55,9 +68,9 @@ function isSocks(proxyUrl) {
 // undici (which node's built-in fetch() is implemented with) only tunnels through http(s)
 // proxies (CONNECT) via ProxyAgent - it has no built-in SOCKS support. agentFor() below can
 // still do SOCKS, since bhttp goes through node's plain http/https modules instead.
-function dispatcherFor(targetUrl) {
+function dispatcherFor(targetUrl, { flip = false } = {}) {
     const { hostname } = new URL(targetUrl);
-    const proxyUrl = resolveProxyUrl(hostname);
+    const proxyUrl = resolveProxyUrl(hostname, { flip });
 
     if (!proxyUrl) {
         return undefined;
@@ -81,9 +94,9 @@ function dispatcherFor(targetUrl) {
 
 // bhttp uses node's raw http/https modules under the hood, which take a plain Agent
 // instance - unlike fetch() above, this does support SOCKS
-function agentFor(targetUrl) {
+function agentFor(targetUrl, { flip = false } = {}) {
     const url = new URL(targetUrl);
-    const proxyUrl = resolveProxyUrl(url.hostname);
+    const proxyUrl = resolveProxyUrl(url.hostname, { flip });
 
     if (!proxyUrl) {
         return undefined;
@@ -105,11 +118,26 @@ function agentFor(targetUrl) {
 }
 
 // drop-in replacement for the global fetch() - importing this as `fetch` transparently
-// proxies requests per config.proxy, if one is configured for the request's host
+// proxies requests per config.proxy, if one is configured for the request's host. Also
+// applies a hard wall-clock ceiling (config.fetch.timeout) to every request: unlike bhttp,
+// fetch() doesn't hang *forever* on a stalled/black-holed connection (undici defaults to a
+// 300s headers/body timeout), but that default is high enough - and, per live testing, a
+// proxy CONNECT tunnel that never completes can still take well over 20s to surface any
+// error - to look like a freeze, especially since reddit/client.js's requestJson() serializes
+// all reddit API calls behind a single-concurrency limiter. Bounding it here covers every
+// fetch()-based call site (reddit's API, and every hosting site's info-lookup) at once.
+// Pass `proxyFlip: true` in options (stripped before reaching fetch()) to use whichever of
+// proxied/direct is NOT what this host would normally get - see resolveProxyUrl().
 async function proxiedFetch(url, options = {}) {
-    const dispatcher = dispatcherFor(String(url));
+    const { proxyFlip, signal, ...fetchOptions } = options;
+    const dispatcher = dispatcherFor(String(url), { flip: proxyFlip });
+    const timeoutSignal = AbortSignal.timeout(config.fetch.timeout);
 
-    return fetch(url, dispatcher ? { ...options, dispatcher } : options);
+    return fetch(url, {
+        ...fetchOptions,
+        ...(dispatcher ? { dispatcher } : {}),
+        signal: signal ? AbortSignal.any([signal, timeoutSignal]) : timeoutSignal,
+    });
 }
 
 module.exports = {

src/reddit/client.js

@@ -111,43 +111,69 @@ async function wait(seconds) {
 async function requestJson(path, params = {}, attempt = 0) {
     const url = buildUrl(path, params);
 
-    return limiter.schedule(async () => {
+    // Only the actual throttled network call belongs inside limiter.schedule() - this has
-        const res = await fetch(url.toString(), { headers: headers() });
+    // maxConcurrent: 1, and every retry path below used to recurse back into requestJson()
-        const contentType = res.headers.get('content-type') || '';
+    // (and so back into limiter.schedule()) from *inside* that same callback. Bottleneck won't
+    // start a second job while the first is still running, and the first can't finish until
+    // its awaited recursive call starts - a guaranteed deadlock the instant a retry was ever
+    // needed (e.g. a single 429), freezing every reddit API call behind it for good. Retrying
+    // now happens here, after the scheduled call has actually returned and freed its slot.
+    let res;
 
-        if (res.status === 429 || res.status === 503) {
+    try {
-            if (attempt >= MAX_RETRIES) {
+        // config.proxy.retryFlipped: once this request has already failed once, retry it via
-                throw new Error(`Reddit rate-limited request after ${attempt} retries (${res.status}) for '${path}'`);
+        // whichever of proxied/direct is NOT what it'd normally get - see proxy.js
-            }
+        res = await limiter.schedule(async () => fetch(url.toString(), {
-
+            headers: headers(),
-            const retryAfter = Number(res.headers.get('retry-after')) || 2 ** attempt;
+            proxyFlip: config.proxy.retryFlipped && attempt > 0,
-
+        }));
-            logger.warn(`Rate limited by reddit (${res.status}) for '${path}', retrying in ${retryAfter}s`);
+    } catch (error) {
-
+        // fetch() has no timeout of its own beyond undici's fairly generous defaults (see
-            await wait(retryAfter);
+        // proxy.js) - this catches both that and genuine network errors (DNS, connection
-
+        // refused/reset, etc.), neither of which was retried before
-            return requestJson(path, params, attempt + 1);
+        if (attempt >= MAX_RETRIES) {
+            throw new Error(`Failed to reach reddit after ${attempt} retries for '${path}': ${error.message}`);
         }
 
-        if (!contentType.includes('json')) {
+        logger.warn(`Failed to reach reddit for '${path}', retrying: ${error.message}`);
-            // reddit serves an HTML interstitial/block page instead of JSON when it decides
-            // the request looks like a bot - this is a distinct failure mode from a genuine
-            // 4xx and should be logged/handled separately rather than treated as "bad request"
-            const hint = cookieHeader() ? ' - session cookie may have expired, refresh config/cookies.js' : ' - likely blocked or interstitial page (consider setting config.reddit.cookies)';
 
-            throw new Error(`Reddit returned non-JSON response (${res.status}, content-type '${contentType || 'none'}') for '${path}'${hint}`);
+        return requestJson(path, params, attempt + 1);
+    }
+
+    const contentType = res.headers.get('content-type') || '';
+
+    if (res.status === 429 || res.status === 503) {
+        if (attempt >= MAX_RETRIES) {
+            throw new Error(`Reddit rate-limited request after ${attempt} retries (${res.status}) for '${path}'`);
         }
 
-        const data = await res.json();
+        const retryAfter = Number(res.headers.get('retry-after')) || 2 ** attempt;
 
-        if (!res.ok) {
+        logger.warn(`Rate limited by reddit (${res.status}) for '${path}', retrying in ${retryAfter}s`);
-            const message = data?.message || data?.reason || JSON.stringify(data).slice(0, 200);
 
-            throw new Error(`Reddit returned ${res.status} for '${path}': ${message}`);
+        await wait(retryAfter);
-        }
 
-        return data;
+        return requestJson(path, params, attempt + 1);
-    });
+    }
+
+    if (!contentType.includes('json')) {
+        // reddit serves an HTML interstitial/block page instead of JSON when it decides
+        // the request looks like a bot - this is a distinct failure mode from a genuine
+        // 4xx and should be logged/handled separately rather than treated as "bad request"
+        const hint = cookieHeader() ? ' - session cookie may have expired, refresh config/cookies.js' : ' - likely blocked or interstitial page (consider setting config.reddit.cookies)';
+
+        throw new Error(`Reddit returned non-JSON response (${res.status}, content-type '${contentType || 'none'}') for '${path}'${hint}`);
+    }
+
+    const data = await res.json();
+
+    if (!res.ok) {
+        const message = data?.message || data?.reason || JSON.stringify(data).slice(0, 200);
+
+        throw new Error(`Reddit returned ${res.status} for '${path}': ${message}`);
+    }
+
+    return data;
 }
 
 async function getUser(username) {