Skip to content

Lower suppressed vocabulary log level #1035

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Lumia1108 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Lower suppressed vocabulary log level #1035

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fdcfd95
Lower suppressed vocabulary log level
Lumia1108 Sep 15, 2026
7adf140
Propagate suppressed document errors
Lumia1108 Sep 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,12 @@ To be released.
objects without the extension. [[#1037], [#1038]]
- Added the `trustEmbeddedObjects` type schema option so embedded metadata
identifiers need not establish trust in linked actors. [[#1037], [#1038]]
- Changed suppressed vocabulary fetch and parsing failures to log at the
warning level so that intentionally handled failures are not reported as
application errors. [[#933], [#1035] by Jae Hui Hong\]

[#933]: https://github.com/fedify-dev/fedify/issues/933
[#1035]: https://github.com/fedify-dev/fedify/pull/1035


Version 2.3.7
Expand Down
8 changes: 8 additions & 0 deletions changes.d/vocab-tools/suppressed-error-log-level.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
links:
'#1035': https://github.com/fedify-dev/fedify/pull/1035
'#933': https://github.com/fedify-dev/fedify/issues/933
---
- Changed suppressed vocabulary fetch and parsing failures to log at the
warning level so that intentionally handled failures are not reported as
application errors. [[#933], [#1035] by Jae Hui Hong]
2 changes: 1 addition & 1 deletion packages/fedify/src/utils/docloader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export function getAuthenticatedDocumentLoader(
validateRedirect: validateUrl,
},
);
return getRemoteDocument(url, response, load);
return getRemoteDocument(url, response, load, options);
}

async function validateUrl(url: string): Promise<void> {
Expand Down
48 changes: 37 additions & 11 deletions packages/vocab-runtime/src/docloader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ export interface DocumentLoaderOptions {
* @since 1.8.0
*/
signal?: AbortSignal;

/**
* Whether to lower error-level logs for recoverable document loading
* failures to warning-level logs. The loader still throws the error.
* @default `false`
*/
suppressError?: boolean;

@dahlia dahlia Sep 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also honor suppressError when URL validation rejects a private address, in both the default and authenticated document loaders. I tested http://127.0.0.1/private with each loader: the accessor returns null with suppressError: true, but Disallowed private URL: {url} is still logged at error level. No network request is made. Please retain the address checks and thrown exceptions, lower this log to warning only for suppressed calls, and add regression coverage for both loaders.

}

/**
Expand Down Expand Up @@ -171,6 +178,7 @@ async function readBoundedText(
* @param url The URL of the document to load.
* @param response The response to get the document from.
* @param fetch The function to fetch the document.
* @param options The options for loading the document.
* @returns The loaded remote document.
* @throws {FetchError} If the response is not OK.
* @internal
Expand All @@ -182,18 +190,31 @@ export async function getRemoteDocument(
url: string,
options?: DocumentLoaderOptions,
) => Promise<RemoteDocument>,
options?: DocumentLoaderOptions,
): Promise<RemoteDocument> {
const documentUrl = response.url === "" ? url : response.url;
const docUrl = new URL(documentUrl);
if (!response.ok) {
logger.error(
"Failed to fetch document: {status} {url} {headers}",
{
status: response.status,
url: documentUrl,
headers: Object.fromEntries(response.headers.entries()),
},
);
if (options?.suppressError) {

@dahlia dahlia Sep 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please apply suppressError to the redirect-limit and redirect-loop branches in getDocumentLoader() as well. Both still call logger.error() unconditionally. I reproduced both cases: Announce.getObject({ suppressError: true }) returns null, but an error-level log is emitted first. Use warning-level logging when suppression is enabled, preserve the existing throws and unsuppressed logging, and add regression coverage for both paths.

logger.warn(
"Failed to fetch document: {status} {url} {headers}",
{
status: response.status,
url: documentUrl,
headers: Object.fromEntries(response.headers.entries()),
},
);
} else {
logger.error(
"Failed to fetch document: {status} {url} {headers}",
{
status: response.status,
url: documentUrl,
headers: Object.fromEntries(response.headers.entries()),
},
);
}

throw new FetchError(
documentUrl,
`HTTP ${response.status}: ${documentUrl}`,
Expand Down Expand Up @@ -242,7 +263,7 @@ export async function getRemoteDocument(
"Found alternate document: {alternateUrl} from {url}",
{ alternateUrl: altUri.href, url: documentUrl },
);
return await fetch(altUri.href);
return await fetch(altUri.href, options);
}
}
}
Expand Down Expand Up @@ -302,7 +323,7 @@ export async function getRemoteDocument(
"Found alternate document: {alternateUrl} from {url}",
{ alternateUrl: attribs.href, url: documentUrl },
);
return await fetch(new URL(attribs.href, docUrl).href);
return await fetch(new URL(attribs.href, docUrl).href, options);
}
}
try {
Expand Down Expand Up @@ -456,7 +477,12 @@ export function getDocumentLoader(
return await load(redirectUrl, options, redirected + 1, visited);
}

const result = await getRemoteDocument(currentUrl, response, load);
const result = await getRemoteDocument(
currentUrl,
response,
load,
options,
);
span.setAttribute("docloader.document_url", result.documentUrl);
if (result.contextUrl != null) {
span.setAttribute("docloader.context_url", result.contextUrl);
Expand Down
Loading
Loading