From c67f6c1d7136ae95d7bf5bc3a04334316d37b835 Mon Sep 17 00:00:00 2001 From: David Tejada Date: Tue, 15 Oct 2024 10:22:23 +0200 Subject: [PATCH] Simplify error handling (#348) --- src/background/actions/openInNewTab.ts | 24 +++---- src/background/background.ts | 57 ++++++++++------ src/background/hints/watchNavigation.ts | 14 ++-- .../messaging/handleRequestFromTalon.ts | 68 ++++++++----------- src/background/utils/clipboard.ts | 54 ++++++--------- src/background/utils/requestAndResponse.ts | 54 +++++++-------- 6 files changed, 130 insertions(+), 141 deletions(-) diff --git a/src/background/actions/openInNewTab.ts b/src/background/actions/openInNewTab.ts index 695d2bee..dd4b2f3d 100644 --- a/src/background/actions/openInNewTab.ts +++ b/src/background/actions/openInNewTab.ts @@ -28,18 +28,14 @@ export async function openInNewTab(urls: string[], active: boolean) { index = 99_999; } - try { - await Promise.all( - urls.map(async (url) => - browser.tabs.create({ - url, - active, - index: index ? index++ : undefined, - openerTabId: currentTab.id, - }) - ) - ); - } catch (error: unknown) { - console.error(error); - } + await Promise.all( + urls.map(async (url) => + browser.tabs.create({ + url, + active, + index: index ? index++ : undefined, + openerTabId: currentTab.id, + }) + ) + ); } diff --git a/src/background/background.ts b/src/background/background.ts index 2602b656..9aa88231 100644 --- a/src/background/background.ts +++ b/src/background/background.ts @@ -8,13 +8,18 @@ import { sendRequestToContent } from "./messaging/sendRequestToContent"; import { contextMenusOnClicked } from "./misc/createContextMenus"; import { initBackgroundScript } from "./setup/initBackgroundScript"; import { browserAction } from "./utils/browserAction"; +import { notify } from "./utils/notify"; // We need to add the listener right away or else clicking the context menu item // while the background script/service worker is inactive might fail. browser.contextMenus.onClicked.addListener(contextMenusOnClicked); (async () => { - await initBackgroundScript(); + try { + await initBackgroundScript(); + } catch (error: unknown) { + console.error(error); + } })(); addEventListener("handle-test-request", handleRequestFromTalon); @@ -24,7 +29,14 @@ browser.runtime.onMessage.addListener(async (message, sender) => { }); browserAction.onClicked.addListener(async () => { - await toggleHintsGlobal(); + try { + await toggleHintsGlobal(); + } catch (error: unknown) { + console.error(error); + if (error instanceof Error) { + await notify(error.message, { type: "error" }); + } + } }); browser.commands.onCommand.addListener(async (internalCommand: string) => { @@ -34,26 +46,33 @@ browser.commands.onCommand.addListener(async (internalCommand: string) => { // No content script. We do nothing. } - if ( - internalCommand === "get-talon-request" || - internalCommand === "get-talon-request-alternative" - ) { - await handleRequestFromTalon(); - } + try { + if ( + internalCommand === "get-talon-request" || + internalCommand === "get-talon-request-alternative" + ) { + await handleRequestFromTalon(); + } - if (internalCommand === "toggle-hints") { - await toggleHintsGlobal(); - } + if (internalCommand === "toggle-hints") { + await toggleHintsGlobal(); + } - if (internalCommand === "disable-hints") { - await updateHintsToggle("global", false); - } + if (internalCommand === "disable-hints") { + await updateHintsToggle("global", false); + } - if (internalCommand === "enable-hints") { - await updateHintsToggle("global", true); - } + if (internalCommand === "enable-hints") { + await updateHintsToggle("global", true); + } - if (internalCommand === "toggle-keyboard-clicking") { - await toggleKeyboardClicking(); + if (internalCommand === "toggle-keyboard-clicking") { + await toggleKeyboardClicking(); + } + } catch (error: unknown) { + console.error(error); + if (error instanceof Error) { + await notify(error.message, { type: "error" }); + } } }); diff --git a/src/background/hints/watchNavigation.ts b/src/background/hints/watchNavigation.ts index 106ad9ce..9251cdd0 100644 --- a/src/background/hints/watchNavigation.ts +++ b/src/background/hints/watchNavigation.ts @@ -49,14 +49,10 @@ export function watchNavigation() { // We also send the frame id in the request as Safari is buggy sending // messages to a specific frame and also sends them to other frames. This // way we can check in the content script. - try { - await sendRequestToContent( - { type: "onCompleted", frameId }, - tabId, - frameId - ); - } catch (error: unknown) { - console.error(error); - } + await sendRequestToContent( + { type: "onCompleted", frameId }, + tabId, + frameId + ); }); } diff --git a/src/background/messaging/handleRequestFromTalon.ts b/src/background/messaging/handleRequestFromTalon.ts index 0139db87..ce72e551 100644 --- a/src/background/messaging/handleRequestFromTalon.ts +++ b/src/background/messaging/handleRequestFromTalon.ts @@ -2,11 +2,10 @@ import { retrieve } from "../../common/storage"; import { promiseWrap } from "../../lib/promiseWrap"; import { type RequestFromTalon } from "../../typings/RequestFromTalon"; import { dispatchCommand } from "../commands/dispatchCommand"; -import { getRequest, postResponse } from "../utils/requestAndResponse"; +import { checkActiveElementIsEditable } from "../utils/checkActiveElementIsEditable"; import { constructTalonResponse } from "../utils/constructTalonResponse"; -import { notify } from "../utils/notify"; +import { getRequest, postResponse } from "../utils/requestAndResponse"; import { shouldTryToFocusDocument } from "../utils/shouldTryToFocusDocument"; -import { checkActiveElementIsEditable } from "../utils/checkActiveElementIsEditable"; import { sendRequestToContent } from "./sendRequestToContent"; let talonIsWaitingForResponse = false; @@ -62,47 +61,36 @@ async function handleDirectClickElementRequest(request: RequestFromTalon) { } export async function handleRequestFromTalon() { - try { - const request = await getRequest(); - if (process.env["NODE_ENV"] !== "production") { - console.log(JSON.stringify(request, null, 2)); - } - - if (!request) { - return; - } + const request = await getRequest(); + if (process.env["NODE_ENV"] !== "production") { + console.log(JSON.stringify(request, null, 2)); + } - talonIsWaitingForResponse = !(request.action.type === "requestTimedOut"); + talonIsWaitingForResponse = !(request.action.type === "requestTimedOut"); - if (request.action.type === "requestTimedOut") return; + if (request.action.type === "requestTimedOut") return; - if (request.action.type === "directClickElement") { - const isRequestHandled = await handleDirectClickElementRequest(request); - if (isRequestHandled) return; - } + if (request.action.type === "directClickElement") { + const isRequestHandled = await handleDirectClickElementRequest(request); + if (isRequestHandled) return; + } - // For these three actions we need to make sure that the document is focused - // or they might fail - if ( - (request.action.type === "setSelectionAfter" || - request.action.type === "setSelectionBefore" || - request.action.type === "tryToFocusElementAndCheckIsEditable") && - (await shouldTryToFocusDocument()) - ) { - const response = constructTalonResponse([{ name: "focusPageAndResend" }]); - await postResponse(response); - return; - } + // For these three actions we need to make sure that the document is focused + // or they might fail + if ( + (request.action.type === "setSelectionAfter" || + request.action.type === "setSelectionBefore" || + request.action.type === "tryToFocusElementAndCheckIsEditable") && + (await shouldTryToFocusDocument()) + ) { + const response = constructTalonResponse([{ name: "focusPageAndResend" }]); + await postResponse(response); + return; + } - const response = await dispatchCommand(request.action); - if (talonIsWaitingForResponse) { - await postResponse(response); - talonIsWaitingForResponse = false; - } - } catch (error: unknown) { - if (error instanceof Error) { - console.error(error); - await notify(error.message, { type: "error" }); - } + const response = await dispatchCommand(request.action); + if (talonIsWaitingForResponse) { + await postResponse(response); + talonIsWaitingForResponse = false; } } diff --git a/src/background/utils/clipboard.ts b/src/background/utils/clipboard.ts index 339cb3a0..1b2283ad 100644 --- a/src/background/utils/clipboard.ts +++ b/src/background/utils/clipboard.ts @@ -64,25 +64,19 @@ async function readClipboardSafari() { } async function readClipboardManifestV3(): Promise { - try { - const hasDocument = await chrome.offscreen.hasDocument(); - if (!hasDocument) { - await chrome.offscreen.createDocument({ - url: urls.offscreenDocument.href, - reasons: [chrome.offscreen.Reason.CLIPBOARD], - justification: "Read the request from Talon from the clipboard", - }); - } - - return await chrome.runtime.sendMessage({ - type: "read-clipboard", - target: "offscreen-doc", + const hasDocument = await chrome.offscreen.hasDocument(); + if (!hasDocument) { + await chrome.offscreen.createDocument({ + url: urls.offscreenDocument.href, + reasons: [chrome.offscreen.Reason.CLIPBOARD], + justification: "Read the request from Talon from the clipboard", }); - } catch (error: unknown) { - console.error(error); } - return undefined; + return chrome.runtime.sendMessage({ + type: "read-clipboard", + target: "offscreen-doc", + }); } async function writeClipboardSafari(text: string) { @@ -100,22 +94,18 @@ async function writeClipboardSafari(text: string) { } async function writeClipboardManifestV3(text: string) { - try { - const hasDocument = await chrome.offscreen.hasDocument(); - if (!hasDocument) { - await chrome.offscreen.createDocument({ - url: urls.offscreenDocument.href, - reasons: [chrome.offscreen.Reason.CLIPBOARD], - justification: "Write the response to Talon to the clipboard", - }); - } - - await chrome.runtime.sendMessage({ - type: "copy-to-clipboard", - target: "offscreen-doc", - text, + const hasDocument = await chrome.offscreen.hasDocument(); + if (!hasDocument) { + await chrome.offscreen.createDocument({ + url: urls.offscreenDocument.href, + reasons: [chrome.offscreen.Reason.CLIPBOARD], + justification: "Write the response to Talon to the clipboard", }); - } catch (error: unknown) { - console.error(error); } + + await chrome.runtime.sendMessage({ + type: "copy-to-clipboard", + target: "offscreen-doc", + text, + }); } diff --git a/src/background/utils/requestAndResponse.ts b/src/background/utils/requestAndResponse.ts index 99f5815b..18aae8ce 100644 --- a/src/background/utils/requestAndResponse.ts +++ b/src/background/utils/requestAndResponse.ts @@ -3,40 +3,40 @@ import type { ResponseToTalon, } from "../../typings/RequestFromTalon"; import { readClipboard, writeClipboard } from "./clipboard"; -import { notify } from "./notify"; /** * Reads and parses the request from the clipboard. */ -export async function getRequest(): Promise { +export async function getRequest() { const clipText = await readClipboard(); - let request: RequestFromTalon; - - if (clipText) { - try { - request = JSON.parse(clipText) as RequestFromTalon; - // This is just to be extra safe - if (request.type !== "request") { - console.error( - 'Error: The message present in the clipboard is not of type "request"' - ); - } - - return request; - } catch (error: unknown) { - // We already check that we are sending valid json in rango-talon, but - // just to be extra sure - if (error instanceof SyntaxError) { - console.error(error); - } - } - } else { - await notify("Unable to read the request present on the clipboard", { - type: "error", - }); + + if (clipText === "") { + throw new Error( + "Clipboard content is not a valid request. Clipboard is empty." + ); } - return undefined; + if (clipText === undefined) { + throw new Error("Unable to read clipboard content."); + } + + try { + const request = JSON.parse(clipText) as RequestFromTalon; + + if (request.type !== "request") { + throw new Error("Clipboard content is not a valid request."); + } + + return request; + } catch (error: unknown) { + // We already check that we are sending valid json in rango-talon, but + // just to be extra sure + if (error instanceof SyntaxError) { + throw new SyntaxError("Clipboard content is not valid JSON."); + } + + throw new Error("Unable to read clipboard content."); + } } /**