fix(tools): isolate read URL reader-mode fallback chain from remote stalls
A stalled Jina reader request shared the overall reader-mode AbortSignal with the downstream trafilatura/lynx/native fallbacks. When Jina hung until the budget timer fired, the shared signal aborted and the catch handler's signal?.throwIfAborted() re-threw before any local fallback ran. - Bound Jina and Parallel extract to their own per-attempt sub-budget (REMOTE_READER_MAX_MS, capped at 10s) so a remote stall cannot consume the whole overall reader-mode budget. - Catch handlers now rethrow only on real userSignal cancellation, not on remote sub-budget or overall budget expiry. - Wrap trafilatura/lynx in their own try/catch so a subprocess failure or abort does not skip the in-process native renderer. - Always attempt the native renderer last: it works on already-loaded HTML with no network or subprocess, so even an exhausted overall budget still yields a result. Fixes #1449
This commit is contained in:
@@ -10,6 +10,7 @@
|
||||
### Fixed
|
||||
|
||||
- Fixed plan-mode re-entry after approval reopening a fresh `local://PLAN.md` instead of the approved titled plan artifact, which could duplicate plan content and fail approval on an existing destination.
|
||||
- Fixed `read` URL reader mode aborting after a stalled Jina request instead of falling back to trafilatura/lynx/native: Jina (and Parallel extract) now have their own per-attempt sub-budget capped at 10s, the catch handler honours only real user cancellation, and the in-process native renderer is always attempted on already-loaded HTML ([#1449](https://github.com/can1357/oh-my-pi/issues/1449))
|
||||
|
||||
## [15.5.6] - 2026-05-27
|
||||
### Added
|
||||
|
||||
@@ -562,9 +562,22 @@ function parseFeedToMarkdown(content: string, maxItems = 10): string {
|
||||
}
|
||||
|
||||
/**
|
||||
* Render HTML to markdown using Parallel, jina, trafilatura, lynx (in order of preference)
|
||||
* Cap on any single remote reader-mode request (Parallel, Jina) so a stalled
|
||||
* remote endpoint cannot consume the whole reader-mode budget and starve the
|
||||
* local fallback renderers (trafilatura, lynx, native). See #1449.
|
||||
*/
|
||||
async function renderHtmlToText(
|
||||
const REMOTE_READER_MAX_MS = 10_000;
|
||||
|
||||
/**
|
||||
* Render HTML to markdown using Parallel, jina, trafilatura, lynx, then the
|
||||
* in-process native converter. The overall `timeout` budget bounds the call,
|
||||
* but remote reader requests are additionally capped at `REMOTE_READER_MAX_MS`
|
||||
* so that a hung remote endpoint cannot prevent local fallbacks from running.
|
||||
* Only a real `userSignal` cancellation aborts the chain — remote per-attempt
|
||||
* timeouts and the overall reader-mode timeout still allow later renderers
|
||||
* (especially the purely-local native converter) to be tried.
|
||||
*/
|
||||
export async function renderHtmlToText(
|
||||
url: string,
|
||||
html: string,
|
||||
timeout: number,
|
||||
@@ -572,14 +585,15 @@ async function renderHtmlToText(
|
||||
userSignal: AbortSignal | undefined,
|
||||
storage: AgentStorage | null,
|
||||
): Promise<{ content: string; ok: boolean; method: string }> {
|
||||
const signal = ptree.combineSignals(userSignal, timeout * 1000);
|
||||
const overallSignal = ptree.combineSignals(userSignal, timeout * 1000);
|
||||
const execOptions = {
|
||||
mode: "group" as const,
|
||||
allowNonZero: true,
|
||||
allowAbort: true,
|
||||
stderr: "full" as const,
|
||||
signal,
|
||||
signal: overallSignal,
|
||||
};
|
||||
const remoteBudgetMs = Math.min(timeout * 1000, REMOTE_READER_MAX_MS);
|
||||
|
||||
// Try Parallel extract first when credentials are configured
|
||||
if (settings.get("providers.parallelFetch") && findParallelApiKey(storage)) {
|
||||
@@ -590,7 +604,7 @@ async function renderHtmlToText(
|
||||
objective: "Extract the main content",
|
||||
excerpts: true,
|
||||
fullContent: false,
|
||||
signal,
|
||||
signal: ptree.combineSignals(userSignal, remoteBudgetMs),
|
||||
},
|
||||
storage,
|
||||
);
|
||||
@@ -602,17 +616,18 @@ async function renderHtmlToText(
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// Parallel extract failed, continue to next method
|
||||
signal?.throwIfAborted();
|
||||
// Parallel extract failed or stalled; honour real cancellation only.
|
||||
userSignal?.throwIfAborted();
|
||||
}
|
||||
}
|
||||
|
||||
// Try jina first (reader API)
|
||||
// Try jina reader API with its own sub-budget so a stall cannot starve
|
||||
// later fallbacks (#1449).
|
||||
try {
|
||||
const jinaUrl = `https://r.jina.ai/${url}`;
|
||||
const response = await fetch(jinaUrl, {
|
||||
headers: { Accept: "text/markdown" },
|
||||
signal,
|
||||
signal: ptree.combineSignals(userSignal, remoteBudgetMs),
|
||||
});
|
||||
if (response.ok) {
|
||||
const content = await response.text();
|
||||
@@ -621,37 +636,50 @@ async function renderHtmlToText(
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// Jina failed, continue to next method
|
||||
signal?.throwIfAborted();
|
||||
// Jina failed or stalled; honour real cancellation only.
|
||||
userSignal?.throwIfAborted();
|
||||
}
|
||||
|
||||
// Try trafilatura (auto-install via uv/pip)
|
||||
const trafilatura = await ensureTool("trafilatura", { signal, silent: true });
|
||||
if (trafilatura) {
|
||||
const result = await ptree.exec([trafilatura, "-u", url, "--output-format", "markdown"], execOptions);
|
||||
if (result.ok && result.stdout.trim().length > 100) {
|
||||
return { content: result.stdout, ok: true, method: "trafilatura" };
|
||||
try {
|
||||
const trafilatura = await ensureTool("trafilatura", { signal: overallSignal, silent: true });
|
||||
if (trafilatura) {
|
||||
const result = await ptree.exec([trafilatura, "-u", url, "--output-format", "markdown"], execOptions);
|
||||
if (result.ok && result.stdout.trim().length > 100) {
|
||||
return { content: result.stdout, ok: true, method: "trafilatura" };
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// trafilatura unavailable or stalled; continue to next method.
|
||||
userSignal?.throwIfAborted();
|
||||
}
|
||||
|
||||
// Try lynx (can't auto-install, system package)
|
||||
const lynx = hasCommand("lynx");
|
||||
if (lynx) {
|
||||
const result = await ptree.exec(["lynx", "-dump", "-nolist", "-width", "250", url], execOptions);
|
||||
if (result.ok) {
|
||||
return { content: result.stdout, ok: true, method: "lynx" };
|
||||
try {
|
||||
const lynx = hasCommand("lynx");
|
||||
if (lynx) {
|
||||
const result = await ptree.exec(["lynx", "-dump", "-nolist", "-width", "250", url], execOptions);
|
||||
if (result.ok) {
|
||||
return { content: result.stdout, ok: true, method: "lynx" };
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// lynx failed or stalled; continue to native converter.
|
||||
userSignal?.throwIfAborted();
|
||||
}
|
||||
|
||||
// Fall back to native converter (fastest, no network/subprocess)
|
||||
// Fall back to native converter (purely local, no network/subprocess).
|
||||
// Always attempted: even if remote renderers and subprocesses were aborted
|
||||
// by the overall reader-mode timeout, this still works on already-loaded
|
||||
// HTML (#1449).
|
||||
try {
|
||||
const content = await htmlToMarkdown(html, { cleanContent: true });
|
||||
if (content.trim().length > 100 && !isLowQualityOutput(content)) {
|
||||
return { content, ok: true, method: "native" };
|
||||
}
|
||||
} catch {
|
||||
// Native converter failed, continue to next method
|
||||
signal?.throwIfAborted();
|
||||
// Native converter failed; nothing else to try.
|
||||
userSignal?.throwIfAborted();
|
||||
}
|
||||
return { content: "", ok: false, method: "none" };
|
||||
}
|
||||
|
||||
@@ -0,0 +1,101 @@
|
||||
import { afterEach, describe, expect, it } from "bun:test";
|
||||
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import { renderHtmlToText } from "@oh-my-pi/pi-coding-agent/tools/fetch";
|
||||
import { hookFetch } from "@oh-my-pi/pi-utils";
|
||||
|
||||
/**
|
||||
* Regression test for #1449: a stalled Jina reader request must not prevent
|
||||
* local fallback renderers (trafilatura/lynx/native) from running within the
|
||||
* overall reader-mode budget.
|
||||
*/
|
||||
describe("renderHtmlToText: jina stall does not starve local fallbacks (#1449)", () => {
|
||||
afterEach(() => {
|
||||
// Nothing to restore — `using` handles fetch hook cleanup per-test.
|
||||
});
|
||||
|
||||
it("falls back to native renderer when jina hangs until aborted", async () => {
|
||||
const settings = Settings.isolated({ "providers.parallelFetch": false });
|
||||
// Substantive HTML so the native converter produces >100 chars and
|
||||
// `isLowQualityOutput` does not reject it.
|
||||
const paragraphs = Array.from({ length: 6 }, (_, i) => `<p>Paragraph number ${i + 1} carries some real content for the article body so the native renderer has enough text to satisfy the length threshold.</p>`).join("");
|
||||
const html = `<!doctype html><html><head><title>Example</title></head><body><article><h1>Example article</h1>${paragraphs}</article></body></html>`;
|
||||
|
||||
using _hook = hookFetch((input, _init, _next) => {
|
||||
const url = String(input);
|
||||
// Hang on the Jina reader endpoint until aborted, mirroring the
|
||||
// real bug: r.jina.ai stalls indefinitely.
|
||||
if (url.startsWith("https://r.jina.ai/")) {
|
||||
return new Promise<Response>((_resolve, reject) => {
|
||||
const signal = _init?.signal;
|
||||
if (!signal) return; // never settles
|
||||
if (signal.aborted) {
|
||||
reject(new DOMException("aborted", "AbortError"));
|
||||
return;
|
||||
}
|
||||
signal.addEventListener("abort", () => {
|
||||
reject(new DOMException("aborted", "AbortError"));
|
||||
});
|
||||
});
|
||||
}
|
||||
return new Response("", { status: 404 });
|
||||
});
|
||||
|
||||
const started = Date.now();
|
||||
// `timeout: 2` keeps the overall budget tight — the test must complete
|
||||
// within ~2s even though Jina would otherwise hang for the full budget.
|
||||
const result = await renderHtmlToText(
|
||||
"https://example.com/article",
|
||||
html,
|
||||
2,
|
||||
settings,
|
||||
undefined,
|
||||
null,
|
||||
);
|
||||
const elapsedMs = Date.now() - started;
|
||||
|
||||
expect(result.ok).toBe(true);
|
||||
// Native converter is the only deterministic local fallback; trafilatura
|
||||
// and lynx may or may not be installed in CI, but native always works.
|
||||
// If trafilatura or lynx happened to succeed first, that's also a valid
|
||||
// non-aborted outcome.
|
||||
expect(["native", "trafilatura", "lynx"]).toContain(result.method);
|
||||
// Must finish well before the overall budget elapses: the remote
|
||||
// sub-budget caps Jina at min(timeout, REMOTE_READER_MAX_MS), so the
|
||||
// remaining ~1s of the 2s budget is enough for the native renderer.
|
||||
expect(elapsedMs).toBeLessThan(2_500);
|
||||
});
|
||||
|
||||
it("re-throws when the user signal is aborted, not when Jina sub-budget expires", async () => {
|
||||
const settings = Settings.isolated({ "providers.parallelFetch": false });
|
||||
const html = "<html><body><p>short</p></body></html>";
|
||||
|
||||
using _hook = hookFetch((_input, init, _next) => {
|
||||
return new Promise<Response>((_resolve, reject) => {
|
||||
const signal = init?.signal;
|
||||
if (!signal) return; // Defensive: never settles otherwise.
|
||||
if (signal.aborted) {
|
||||
reject(new DOMException("aborted", "AbortError"));
|
||||
return;
|
||||
}
|
||||
signal.addEventListener("abort", () => {
|
||||
reject(new DOMException("aborted", "AbortError"));
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
const controller = new AbortController();
|
||||
const pending = renderHtmlToText(
|
||||
"https://example.com/article",
|
||||
html,
|
||||
30,
|
||||
settings,
|
||||
controller.signal,
|
||||
null,
|
||||
).catch(err => err);
|
||||
|
||||
controller.abort();
|
||||
const outcome = await pending;
|
||||
expect(outcome).toBeInstanceOf(Error);
|
||||
expect((outcome as Error).name === "AbortError" || (outcome as Error).message.toLowerCase().includes("abort")).toBe(true);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user