diff --git a/src/extensionPoints/menuItemExtension.ts b/src/extensionPoints/menuItemExtension.ts index 96a5b6eecb..507b9f1a15 100644 --- a/src/extensionPoints/menuItemExtension.ts +++ b/src/extensionPoints/menuItemExtension.ts @@ -71,6 +71,7 @@ import sanitize from "@/utils/sanitize"; import { EXTENSION_POINT_DATA_ATTR } from "@/common"; import BackgroundLogger from "@/telemetry/BackgroundLogger"; import reportError from "@/telemetry/reportError"; +import pluralize from "@/utils/pluralize"; interface ShadowDOM { mode?: "open" | "closed"; @@ -726,18 +727,13 @@ export abstract class MenuItemExtensionPoint extends ExtensionPoint 0) { - // Report the error to the user. Don't report to to the telemetry service since we already reported - // above in the loop - console.warn(`An error occurred adding ${errors.length} menu item(s)`, { - errors, + const subject = pluralize(errors.length, "the button", "$$ buttons"); + const message = `An error occurred adding ${subject}`; + console.warn(message, { errors }); + this.notifyError({ + message, + reportError: false, // We already reported it in the loop above }); - if (errors.length === 1) { - this.notifyError("An error occurred adding the menu item/button"); - } else { - this.notifyError( - `An error occurred adding ${errors.length} menu items` - ); - } } } } diff --git a/src/extensionPoints/triggerExtension.ts b/src/extensionPoints/triggerExtension.ts index b770f61841..16b7f24601 100644 --- a/src/extensionPoints/triggerExtension.ts +++ b/src/extensionPoints/triggerExtension.ts @@ -48,10 +48,11 @@ import apiVersionOptions from "@/runtime/apiVersionOptions"; import { blockList } from "@/blocks/util"; import { makeServiceContext } from "@/services/serviceUtils"; import { mergeReaders } from "@/blocks/readers/readerUtils"; -import { asyncForEach, PromiseCancelled, sleep } from "@/utils"; +import { PromiseCancelled, sleep } from "@/utils"; import initialize from "@/vendors/initialize"; import { $safeFind } from "@/helpers"; import BackgroundLogger from "@/telemetry/BackgroundLogger"; +import pluralize from "@/utils/pluralize"; export type TriggerConfig = { action: BlockPipeline | BlockConfig; @@ -266,6 +267,7 @@ export abstract class TriggerExtensionPoint extends ExtensionPoint { const reader = await this.defaultReader(); const readerContext = await reader.read(root); @@ -324,14 +330,34 @@ export abstract class TriggerExtensionPoint extends ExtensionPoint>): void { - const errors = compact( - results.flatMap((x) => (x.status === "fulfilled" ? x.value : [x.reason])) + private async runTriggersAndNotify(...roots: ReaderRoot[]): Promise { + const promises = roots.map(async (root) => this.runTrigger(root)); + const results = await Promise.allSettled(promises); + const errors = results.flatMap((x) => + // `runTrigger` fulfills with list of extension error from extension, or rejects on other error, e.g., reader + // error from the extension point. + x.status === "fulfilled" ? x.value : x.reason ); - if (errors.length > 0) { - console.debug("Trigger errors", errors); - notify.error(`An error occurred running ${errors.length} triggers(s)`); + + TriggerExtensionPoint.notifyErrors(errors); + } + + /** + * Show notification for errors to the user. Caller is responsible for sending error telemetry. + * @param errors + */ + static notifyErrors(errors: unknown[]): void { + if (errors.length === 0) { + return; } + + const subject = pluralize(errors.length, "a trigger", "$$ triggers"); + const message = `An error occurred running ${subject}`; + console.debug(message, { errors }); + notify.error({ + message, + reportError: false, + }); } private async getRoot(): Promise> { @@ -372,9 +398,7 @@ export abstract class TriggerExtensionPoint extends ExtensionPoint { const $root = await this.getRoot(); - await Promise.allSettled( - $root.toArray().flatMap(async (root) => this.runTrigger(root)) - ); + await this.runTriggersAndNotify(...$root); }; void interval({ @@ -401,23 +425,14 @@ export abstract class TriggerExtensionPoint extends ExtensionPoint { - const errors = await this.runTrigger(element as HTMLElement); - if (errors.length > 0) { - console.error("An error occurred while running a trigger", { - errors, - }); - notify.error("An error occurred while running a trigger"); - } + (index, element: HTMLElement) => { + void this.runTriggersAndNotify(element); }, // `target` is a required option { target: document } @@ -434,18 +449,10 @@ export abstract class TriggerExtensionPoint extends ExtensionPoint { - void asyncForEach(entries, async (entry) => { - if (!entry.isIntersecting) { - return; - } - - const errors = await this.runTrigger(entry.target as HTMLElement); - if (errors.length > 0) { - const message = "An error occurred while running a trigger"; - console.error(message, { errors }); - notify.error({ message, error: errors[0] }); - } - }); + const roots = entries + .filter((x) => x.isIntersecting) + .map((x) => x.target as HTMLElement); + void this.runTriggersAndNotify(...roots); }, { root: null, @@ -546,10 +553,7 @@ export abstract class TriggerExtensionPoint extends ExtensionPoint this.runTrigger(root)) - ); - TriggerExtensionPoint.notifyErrors(promises); + await this.runTriggersAndNotify(...$root); break; } diff --git a/src/telemetry/BackgroundLogger.ts b/src/telemetry/BackgroundLogger.ts index 6de7aecb8e..a944795343 100644 --- a/src/telemetry/BackgroundLogger.ts +++ b/src/telemetry/BackgroundLogger.ts @@ -17,7 +17,7 @@ import { Logger, MessageContext } from "@/core"; import { JsonObject } from "type-fest"; -import { getErrorMessage, isConnectionError } from "@/errors"; +import { isConnectionError, selectError } from "@/errors"; import { getContextName, isContentScript } from "webext-detect-page"; import { showConnectionLost } from "@/contentScript/connection"; import { serializeError } from "serialize-error"; @@ -71,8 +71,8 @@ class BackgroundLogger implements Logger { } async error(error: unknown, data: JsonObject): Promise { - console.error("An error occurred: %s", getErrorMessage(error), { - error, + // Use selectError which returns an error object ot avoid user-defined format strings + console.error(selectError(error), { context: this.context, data, }); diff --git a/src/utils/notify.tsx b/src/utils/notify.tsx index c098afb36d..6fc6c203cd 100644 --- a/src/utils/notify.tsx +++ b/src/utils/notify.tsx @@ -185,7 +185,11 @@ export function notifyResult( extensionId: string, { message, config: { className } }: MessageConfig ): void { - showNotification({ message, type: className as NotificationType }); + showNotification({ + message, + type: className as NotificationType, + reportError: false, + }); } // Private method to prevent adding logic to the `notify.*` helpers. diff --git a/src/utils/pluralize.ts b/src/utils/pluralize.ts new file mode 100644 index 0000000000..510ced9155 --- /dev/null +++ b/src/utils/pluralize.ts @@ -0,0 +1,22 @@ +// From: https://github.com/refined-github/refined-github/blob/4232313404c9d4afaa711217ce539680b7e3799e/source/helpers/pluralize.ts + +function regular(single: string): string { + return single + "s"; +} + +export default function pluralize( + count: number, + single: string, + plural = regular(single), + zero?: string +): string { + if (count === 0 && zero) { + return zero.replace("$$", "0"); + } + + if (count === 1) { + return single.replace("$$", "1"); + } + + return plural.replace("$$", String(count)); +}