diff --git a/CHANGES.md b/CHANGES.md index 6eae6a6a6..5c06df1a6 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -25,6 +25,8 @@ To be released. ### @fedify/cli + - Changed debug logs for suppressed recursive lookup errors to preserve the + original error as a cause alongside the failed target. [[#901], [#1265]] - Changed the `fedify nodeinfo` favicon selector to pick a usable bitmap icon more often. It now skips SVG icons declared with `type="image/svg+xml"`, not just those with a `.svg` URL, and it considers @@ -33,7 +35,9 @@ To be released. [[#893], [#1187] by Lee Jeongmin\] [#893]: https://github.com/fedify-dev/fedify/issues/893 +[#901]: https://github.com/fedify-dev/fedify/issues/901 [#1187]: https://github.com/fedify-dev/fedify/pull/1187 +[#1265]: https://github.com/fedify-dev/fedify/pull/1265 ### @fedify/debugger diff --git a/changes.d/cli/recursive-lookup-debug-errors.md b/changes.d/cli/recursive-lookup-debug-errors.md new file mode 100644 index 000000000..9fd680f97 --- /dev/null +++ b/changes.d/cli/recursive-lookup-debug-errors.md @@ -0,0 +1,7 @@ +--- +links: + '#1265': https://github.com/fedify-dev/fedify/pull/1265 + '#901': https://github.com/fedify-dev/fedify/issues/901 +--- + - Changed debug logs for suppressed recursive lookup errors to preserve the + original error as a cause alongside the failed target. [[#901], [#1265]] diff --git a/packages/cli/src/lookup.test.ts b/packages/cli/src/lookup.test.ts index 8b12b8ba0..c94547a0b 100644 --- a/packages/cli/src/lookup.test.ts +++ b/packages/cli/src/lookup.test.ts @@ -17,6 +17,7 @@ import { getContextLoader } from "./docloader.ts"; import { runCli } from "./runner.ts"; import { authorizedFetchOption, + classifyRecursiveLookupFailure, collectAsyncItems, collectRecursiveObjects, getLookupFailureHint, @@ -24,6 +25,7 @@ import { getRecursiveTargetId, isDocumentLoaderTimeoutError, lookupCommand, + printRecursiveLookupFailure, RecursiveLookupError, runLookup, shouldPrintLookupFailureHint, @@ -777,6 +779,237 @@ test("getRecursiveTargetId - returns null for unknown recurse property", () => { ); }); +const recursiveFailureOptions = { + allowPrivateAddress: false, + authorizedFetch: false, +}; + +function privateContextError(url: string): Error { + return Object.assign( + new Error(`Could not retrieve a valid JSON-LD object. URL: "${url}"`), + { name: "jsonld.InvalidUrl", details: { url } }, + ); +} + +test("classifyRecursiveLookupFailure - preserves target and timeout presentation", () => { + const target = "https://example.com/parent"; + const timeout = new TimeoutError("timed out"); + const cases = [ + // Raw fallback is also supported when no recursive target was retained. + { error: timeout, target: undefined, presentation: "generic" }, + { + error: new RecursiveLookupError(target, undefined, { cause: timeout }), + target, + presentation: "generic", + }, + { + error: new RecursiveLookupError(target, { + error: timeout, + source: "object", + }), + target, + presentation: "target", + }, + { + error: new RecursiveLookupError(target, { + error: timeout, + source: "context", + }, { cause: privateContextError("http://localhost/context") }), + target, + presentation: "target", + }, + ]; + for (const { error, target, presentation } of cases) { + const failure = classifyRecursiveLookupFailure( + error, + recursiveFailureOptions, + ); + assert.equal(failure.kind, "timeout"); + assert.equal(failure.target, target); + assert.equal(failure.presentation, presentation); + assert.equal(failure.suppressible, true); + assert.deepEqual(failure.hint, { type: "none" }); + assert.equal( + failure.error, + presentation === "generic" && error.cause != null ? error.cause : error, + ); + } +}); + +test("classifyRecursiveLookupFailure - distinguishes private targets and contexts", () => { + const target = "http://localhost/parent"; + const contextUrl = "http://127.0.0.1/context"; + const contextError = privateContextError(contextUrl); + const recorded = { error: contextError, source: "context" as const }; + const wrapped = new RecursiveLookupError(target, recorded); + const blocked = classifyRecursiveLookupFailure( + wrapped, + recursiveFailureOptions, + ); + assert.equal(blocked.kind, "context"); + assert.deepEqual(blocked.hint, { type: "private-address" }); + const allowed = classifyRecursiveLookupFailure(wrapped, { + ...recursiveFailureOptions, + allowPrivateAddress: true, + }); + assert.equal(allowed.kind, "context"); + assert.deepEqual(allowed.hint, { + type: "private-context", + url: new URL(contextUrl), + }); + // The actual jsonld path retains the loader error but throws its wrapper. + const raw = new RecursiveLookupError(target, { + error: new UrlError("Invalid or private address", { reason: "disallowed" }), + source: "context", + }, { cause: contextError }); + const failure = classifyRecursiveLookupFailure(raw, recursiveFailureOptions); + assert.equal(failure.target, target); + assert.equal(failure.source, "context"); + assert.equal(failure.kind, "context"); + assert.equal(failure.presentation, "generic"); + assert.equal(failure.error, contextError); + assert.equal(failure.suppressible, true); + assert.deepEqual(failure.hint, allowed.hint); + + const http = new FetchError(target, `HTTP 500: ${target}`); + const allowedHttp = classifyRecursiveLookupFailure( + new RecursiveLookupError(target, { error: http, source: "object" }), + { ...recursiveFailureOptions, allowPrivateAddress: true }, + ); + assert.equal(allowedHttp.kind, "fetch-or-parse"); + assert.deepEqual(allowedHttp.hint, { + type: "diagnostic", + message: `HTTP 500 from ${target}.`, + }); + const privateAddress = classifyRecursiveLookupFailure( + new RecursiveLookupError("https://example.com/parent", { + error: new UrlError("Invalid or private address"), + source: "object", + }), + recursiveFailureOptions, + ); + assert.equal(privateAddress.kind, "private-address"); + assert.deepEqual(privateAddress.hint, { + type: "diagnostic", + message: "Invalid or private address", + }); +}); + +test("classifyRecursiveLookupFailure - preserves recorded and raw DNS hints", () => { + const target = "https://example.com/parent"; + const error = new UrlError("Invalid or private address", { reason: "dns" }); + for (const authorizedFetch of [false, true]) { + const options = { ...recursiveFailureOptions, authorizedFetch }; + const raw = classifyRecursiveLookupFailure( + new RecursiveLookupError(target, { error, source: "object" }, { + cause: error, + }), + options, + ); + assert.equal(raw.kind, "fetch-or-parse"); + assert.equal(raw.presentation, "generic"); + assert.deepEqual(raw.hint, { type: "dns" }); + const recorded = classifyRecursiveLookupFailure( + new RecursiveLookupError(target, { error, source: "object" }), + options, + ); + assert.equal(recorded.kind, "fetch-or-parse"); + assert.deepEqual(recorded.hint, { + type: "diagnostic", + message: + "Could not resolve the host in the URL. Check the URL and your network connection.", + }); + } +}); + +test("classifyRecursiveLookupFailure - retains context, HTTP and portable diagnostics", () => { + const target = "https://example.com/parent"; + for (const status of [401, 403, 404]) { + const error = new FetchError(target, `HTTP ${status}: ${target}`); + for (const source of ["object", "context"] as const) { + for (const authorizedFetch of [false, true]) { + const failure = classifyRecursiveLookupFailure( + new RecursiveLookupError(target, { error, source }), + { ...recursiveFailureOptions, authorizedFetch }, + ); + assert.equal( + failure.kind, + source === "context" ? "context" : "fetch-or-parse", + ); + assert.equal(failure.source, source); + assert.equal(failure.suppressible, true); + assert.deepEqual(failure.hint, { + type: "diagnostic", + message: `HTTP ${status} from ${target}.` + + (source === "object" && !authorizedFetch + ? " It may be a private object. Try with -a/--authorized-fetch." + : ""), + }); + } + } + } + const parse = new SyntaxError("Unexpected token"); + const context = classifyRecursiveLookupFailure( + new RecursiveLookupError(target, { error: parse, source: "context" }, { + cause: new Error("JSON-LD processing failed"), + }), + recursiveFailureOptions, + ); + assert.equal(context.kind, "context"); + assert.equal(context.presentation, "generic"); + assert.deepEqual(context.hint, { type: "authorized-fetch" }); + const portable = classifyRecursiveLookupFailure( + new RecursiveLookupError(target, { + error: undefined, + source: "portable", + id: "ap://did:key:example/parent", + problem: "no-gateway", + }), + recursiveFailureOptions, + ); + assert.equal(portable.kind, "fetch-or-parse"); + assert.equal(portable.source, "portable"); + assert.equal(portable.suppressible, true); + assert.deepEqual(portable.hint, { + type: "diagnostic", + message: + "The portable ID ap://did:key:example/parent has no @gateway location hints. Use the --gateway option to look it up.", + }); +}); + +test("classifyRecursiveLookupFailure - preserves unknown and authenticated hints", () => { + const target = "https://example.com/parent"; + for (const authorizedFetch of [false, true]) { + const options = { ...recursiveFailureOptions, authorizedFetch }; + // No evidence means a null result, not proof of an HTTP 404. + const missing = classifyRecursiveLookupFailure( + new RecursiveLookupError(target), + options, + ); + assert.equal(missing.kind, "not-found"); + assert.equal(missing.suppressible, true); + assert.deepEqual(missing.hint, { + type: authorizedFetch ? "none" : "authorized-fetch", + }); + const raw = classifyRecursiveLookupFailure( + new RecursiveLookupError(target, undefined, { cause: "parse failed" }), + options, + ); + assert.equal(raw.kind, "fetch-or-parse"); + assert.equal(raw.error, "parse failed"); + assert.equal(raw.target, target); + assert.deepEqual(raw.hint, { + type: authorizedFetch ? "suppress-errors" : "authorized-fetch", + }); + } + const undefinedCause = classifyRecursiveLookupFailure( + new RecursiveLookupError(target, undefined, { cause: undefined }), + recursiveFailureOptions, + ); + assert.equal(undefinedCause.presentation, "target"); + assert.equal(undefinedCause.kind, "not-found"); +}); + test("getLookupFailureHint - suggests private-address for UrlError", () => { assert.equal( getLookupFailureHint( @@ -1168,6 +1401,97 @@ async function captureStderr( } } +test("printRecursiveLookupFailure - preserves timeout targets and authenticated hints", async () => { + const target = "https://lookup.test/parent"; + const timeout = new TimeoutError("timed out"); + for (const recorded of [false, true]) { + const failure = classifyRecursiveLookupFailure( + new RecursiveLookupError( + target, + recorded ? { error: timeout, source: "object" } : undefined, + { cause: timeout }, + ), + recursiveFailureOptions, + ); + const headlines: string[] = []; + const { stderr } = await captureStderr(() => { + printRecursiveLookupFailure(failure, { + fail: (headline) => { + headlines.push(headline); + }, + }, 0.2); + return Promise.resolve(); + }); + assert.equal(headlines.length, 1); + assert.match(headlines[0], /Request timed out after 0\.2 seconds/); + if (recorded) assert.match(headlines[0], /for: .*lookup\.test\/parent/); + else assert.equal(headlines[0], "Request timed out after 0.2 seconds."); + assert.match(stderr, /--timeout/); + } + const failure = classifyRecursiveLookupFailure( + new RecursiveLookupError(target, undefined, { + cause: new Error("parse failed"), + }), + { ...recursiveFailureOptions, authorizedFetch: true }, + ); + const headlines: string[] = []; + const { stderr } = await captureStderr(() => { + printRecursiveLookupFailure(failure, { + fail: (headline) => { + headlines.push(headline); + }, + }); + return Promise.resolve(); + }); + assert.deepEqual(headlines, ["Failed to recursively fetch object."]); + assert.match(stderr, /--suppress-errors/); + assert.doesNotMatch(stderr, /--authorized-fetch/); +}); + +test("runLookup - reports recorded recursive timeouts with the target", async () => { + const testDir = getTestOutputDir("runlookup_recursive_timeout"); + const testFile = `${testDir}/out.jsonl`; + await mkdir(testDir, { recursive: true }); + try { + const rootUrl = "https://lookup.test/root"; + const target = "https://lookup.test/parent"; + const root = new Note({ + id: new URL(rootUrl), + replyTarget: new URL(target), + }); + const { result: exitCode, stderr } = await captureStderr(() => + runLookupAndCaptureExitCode( + createLookupRunCommand({ + urls: [rootUrl], + recurse: "replyTarget", + recurseDepth: 20, + timeout: 0.2, + output: testFile, + }), + { + lookupObject: (url) => { + if (String(url) === rootUrl) return Promise.resolve(root); + throw new TimeoutError("timed out"); + }, + }, + ) + ); + assert.equal(exitCode, 1); + assert.match( + stderr, + /Request timed out after 0\.2 seconds for: .*lookup\.test\/parent/, + ); + assert.match(stderr, /--timeout/); + assert.doesNotMatch(stderr, /--authorized-fetch/); + assert.deepEqual( + extractIdsFromRawOutput(await readFile(testFile, "utf8")), + [rootUrl], + ); + } finally { + await rm(testDir, { recursive: true }); + } +}); + test("runLookup - prints DNS guidance for a thrown recursive lookup failure", async () => { const testDir = "./test_output_runlookup_recursive_dns"; await mkdir(testDir, { recursive: true }); @@ -1431,6 +1755,43 @@ test("runLookup - keeps recursive private contexts blocked", async () => { } }); +test("runLookup - suppresses private context failures in either presentation order", async () => { + for (const reverse of [false, true]) { + const testDir = getTestOutputDir( + `runlookup_private_context_suppressed_${reverse}`, + ); + const testFile = `${testDir}/out.jsonl`; + await mkdir(testDir, { recursive: true }); + try { + await withRecursiveLookupServer( + { replyContextPath: "/contexts/reply" }, + async ({ rootUrl, requestedPaths }) => { + const { result: exitCode, stderr } = await captureStderr(() => + runLookupAndCaptureExitCode(createLookupRunCommand({ + urls: [rootUrl.href], + recurse: "replyTarget", + recurseDepth: 20, + allowPrivateAddress: true, + suppressErrors: true, + reverse, + output: testFile, + })) + ); + assert.equal(exitCode, 0); + assert.deepEqual(requestedPaths, ["/notes/1", "/notes/0"]); + assert.doesNotMatch(stderr, /always blocked|--authorized-fetch/); + assert.deepEqual( + extractIdsFromRawOutput(await readFile(testFile, "utf8")), + [rootUrl.href], + ); + }, + ); + } finally { + await rm(testDir, { recursive: true }); + } + } +}); + test("runLookup - reverses output order in default multi-input mode", async () => { const testDir = getTestOutputDir("runlookup_default_reverse"); const testFile = `${testDir}/out.jsonl`; diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index ee2b72959..0d52cc994 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -88,8 +88,8 @@ export class RecursiveLookupError extends Error { target: string; /** Why the target could not be fetched, if known. */ failure?: LookupFailure; - constructor(target: string, failure?: LookupFailure) { - super(`Failed to recursively fetch object: ${target}`); + constructor(target: string, failure?: LookupFailure, options?: ErrorOptions) { + super(`Failed to recursively fetch object: ${target}`, options); this.name = "RecursiveLookupError"; this.target = target; this.failure = failure; @@ -478,6 +478,192 @@ function printLookupFailureHint( } } +/** A follow-up lookup failure, classified without changing loader policy. */ +export interface RecursiveLookupFailure { + /** The recursive object target, when the lookup retained it. */ + readonly target?: string; + /** + * `not-found` means no object and no recorded evidence, not an HTTP 404. + * Context failures retain their source even when classified as timeouts + * or private-address rejections. + */ + readonly kind: + | "timeout" + | "private-address" + | "not-found" + | "context" + | "fetch-or-parse"; + readonly source?: LookupFailure["source"]; + /** + * Whether suppression can return the already collected prefix. This model + * covers follow-up fetches only, never initial lookups or output writes. + */ + readonly suppressible: boolean; + /** Preserve generic headlines for errors that previously escaped directly. */ + readonly presentation: "target" | "generic"; + /** The error to log, preserving the original generic error when available. */ + readonly error: unknown; + readonly hint: + | { readonly type: "private-address" } + | { readonly type: "private-context"; readonly url: URL } + | { readonly type: "dns" } + | { readonly type: "authorized-fetch" } + | { readonly type: "suppress-errors" } + | { readonly type: "diagnostic"; readonly message: string } + | { readonly type: "none" }; +} + +function isSuppressibleRecursiveFailure( + kind: RecursiveLookupFailure["kind"], +): boolean { + switch (kind) { + case "timeout": + case "private-address": + case "not-found": + case "context": + case "fetch-or-parse": + return true; + } +} + +/** + * Classifies recursive follow-up failures and selects their existing hints. + * @param error The error from collecting a recursive chain. + * @param options The policies that affect hints, rather than loader behavior. + * @returns A failure that can be rendered without inspecting its error again. + */ +export function classifyRecursiveLookupFailure( + error: unknown, + options: { allowPrivateAddress: boolean; authorizedFetch: boolean }, +): RecursiveLookupFailure { + const wrapped = error instanceof RecursiveLookupError ? error : undefined; + const failure = wrapped?.failure; + const recordedTimeout = failure?.error instanceof TimeoutError; + const presentation = wrapped != null && + (wrapped.cause == null || recordedTimeout) + ? "target" + : "generic"; + const underlyingError = presentation === "target" + ? failure?.error + : wrapped?.cause ?? error; + const timeout = underlyingError instanceof TimeoutError || recordedTimeout; + const privateContextUrl = presentation === "generic" || + failure?.source === "context" + ? getPrivateContextUrl(underlyingError) + : null; + const blockedTarget = presentation === "target" && + !options.allowPrivateAddress && wrapped != null && + isPrivateAddressTarget(wrapped.target); + + let hint: RecursiveLookupFailure["hint"] = { type: "none" }; + if (!timeout) { + if (presentation === "target") { + if (blockedTarget) hint = { type: "private-address" }; + else if (privateContextUrl != null) { + hint = { type: "private-context", url: privateContextUrl }; + } else if (failure != null) { + hint = { + type: "diagnostic", + message: describeLookupFailure(failure, options.authorizedFetch) + .message, + }; + } else if (!options.authorizedFetch) { + hint = { type: "authorized-fetch" }; + } + } else if (privateContextUrl != null) { + hint = { type: "private-context", url: privateContextUrl }; + } else { + const legacyHint = getLookupFailureHint(underlyingError, { + recursive: true, + }); + hint = legacyHint === "authorized-fetch" + ? { type: options.authorizedFetch ? "suppress-errors" : legacyHint } + : { type: legacyHint === "dns" ? "dns" : "private-address" }; + } + } + + const kind: RecursiveLookupFailure["kind"] = timeout + ? "timeout" + : privateContextUrl != null + ? "context" + // FetchError messages include their URL; "localhost" in that URL is + // not evidence that the loader blocked the address. + : blockedTarget || + !(underlyingError instanceof FetchError) && + isPrivateAddressError(underlyingError) + ? "private-address" + : failure?.source === "context" + ? "context" + : presentation === "target" && failure == null + ? "not-found" + : "fetch-or-parse"; + return { + target: wrapped?.target, + kind, + source: failure?.source, + suppressible: isSuppressibleRecursiveFailure(kind), + presentation, + error: presentation === "generic" ? underlyingError : error, + hint, + }; +} + +/** + * Renders a classified recursive failure without reinterpreting its error. + * @param failure The classified follow-up failure. + * @param spinner The spinner that displays the failure headline. + * @param timeoutSeconds The requested timeout, in seconds. + * @internal + */ +export function printRecursiveLookupFailure( + failure: RecursiveLookupFailure, + spinner: { fail: (text: string) => void }, + timeoutSeconds?: number, +): void { + const target = failure.presentation === "target" ? failure.target : undefined; + if (failure.kind === "timeout") { + handleTimeoutError(spinner, timeoutSeconds, target); + return; + } + spinner.fail( + target == null + ? "Failed to recursively fetch object." + : `Failed to recursively fetch object: ${colors.red(target)}.`, + ); + switch (failure.hint.type) { + case "private-address": + printRecursivePrivateAddressHint(); + return; + case "private-context": + printRecursivePrivateContextHint(failure.hint.url); + return; + case "dns": + printError( + message`DNS lookup failed. Check the hostname and network connectivity.`, + ); + return; + case "authorized-fetch": + printError( + message`It may be a private object. Try with ${ + optionNames(["-a", "--authorized-fetch"]) + }.`, + ); + return; + case "suppress-errors": + printError( + message`Use the ${ + optionNames(["-S", "--suppress-errors"]) + } option to suppress partial errors.`, + ); + return; + case "diagnostic": + printError(message`${text(failure.hint.message)}`); + return; + case "none": + return; + } +} + /** * Gets the next recursion target URL from an ActivityPub object. */ @@ -914,13 +1100,13 @@ export async function runLookup( : {}), }); if (result.object != null) return result.object; - if ( - result.thrownError != null && - !(result.failure?.error instanceof TimeoutError) - ) { - throw result.thrownError; - } - throw new RecursiveLookupError(target, result.failure); + throw new RecursiveLookupError( + target, + result.failure, + result.thrownError == null + ? undefined + : { cause: result.thrownError }, + ); }, { suppressErrors: command.suppressErrors, visited }, ); @@ -947,65 +1133,15 @@ export async function runLookup( return; } } + const failure = classifyRecursiveLookupFailure(error, { + allowPrivateAddress: command.allowPrivateAddress, + authorizedFetch: authLoader != null, + }); logger.error( "Failed to recursively fetch an object in chain: {error}", - { - error, - }, + { error: failure.error }, ); - if (error instanceof TimeoutError) { - handleTimeoutError(spinner, command.timeout); - } else if ( - error instanceof RecursiveLookupError && - error.failure?.error instanceof TimeoutError - ) { - handleTimeoutError(spinner, command.timeout, error.target); - } else if (error instanceof RecursiveLookupError) { - spinner.fail( - `Failed to recursively fetch object: ${colors.red(error.target)}.`, - ); - const privateContextUrl = error.failure?.source === "context" - ? getPrivateContextUrl(error.failure.error) - : null; - if ( - !command.allowPrivateAddress && - isPrivateAddressTarget(error.target) - ) { - printRecursivePrivateAddressHint(); - } else if (privateContextUrl != null) { - printRecursivePrivateContextHint(privateContextUrl); - } else if (error.failure != null) { - const diagnostic = describeLookupFailure( - error.failure, - authLoader != null, - ); - printError(message`${text(diagnostic.message)}`); - } else if (authLoader == null) { - printError( - message`It may be a private object. Try with ${ - optionNames(["-a", "--authorized-fetch"]) - }.`, - ); - } - } else { - spinner.fail("Failed to recursively fetch object."); - const privateContextUrl = getPrivateContextUrl(error); - if (privateContextUrl != null) { - printRecursivePrivateContextHint(privateContextUrl); - await finalizeAndExit(1); - return; - } - const hint = getLookupFailureHint(error, { recursive: true }); - if (shouldSuggestSuppressErrorsForLookupFailure(authLoader, hint)) { - printError( - message`Use the ${ - optionNames(["-S", "--suppress-errors"]) - } option to suppress partial errors.`, - ); - } else { - printLookupFailureHint(authLoader, error, { recursive: true }); - } - } + printRecursiveLookupFailure(failure, spinner, command.timeout); await finalizeAndExit(1); return; }