Skip to content

Commit

Permalink
#3940: Drop "receiving end does not exist" errors from "connection lo…
Browse files Browse the repository at this point in the history
…st" check (#3941)

Co-authored-by: Todd Schiller <todd.schiller@gmail.com>
  • Loading branch information
fregante and twschiller committed Aug 2, 2022
1 parent 1cc95b6 commit 3c62042
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 60 deletions.
16 changes: 1 addition & 15 deletions src/chrome.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@
*/

import { isExtensionContext } from "webext-detect-page";
import { expectContext, forbidContext } from "@/utils/expectContext";
import { forbidContext } from "@/utils/expectContext";
import { JsonValue } from "type-fest";
import { once } from "lodash";

// eslint-disable-next-line prefer-destructuring -- It breaks EnvironmentPlugin
const CHROME_EXTENSION_ID = process.env.CHROME_EXTENSION_ID;
Expand Down Expand Up @@ -139,16 +138,3 @@ export async function onTabClose(watchedTabId: number): Promise<void> {
browser.tabs.onRemoved.addListener(listener);
});
}

export const onContextInvalidated = once(async (): Promise<void> => {
expectContext("extension");

return new Promise((resolve) => {
const interval = setInterval(() => {
if (!chrome.runtime?.id) {
resolve();
clearInterval(interval);
}
}, 200);
});
});
14 changes: 8 additions & 6 deletions src/contentScript/contentScript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,26 +42,28 @@ import { ENSURE_CONTENT_SCRIPT_READY } from "@/messaging/constants";
// eslint-disable-next-line import/no-restricted-paths -- Custom devTools mechanism to transfer data
import { addListenerForUpdateSelectedElement } from "@/pageEditor/getSelectedElement";
import { initToaster } from "@/utils/notify";
import { isConnectionError } from "@/errors/errorHelpers";
import { showConnectionLost } from "@/contentScript/connection";
import {
isContextInvalidatedError,
notifyContextInvalidated,
} from "@/errors/contextInvalidated";
import { initPartnerIntegrations } from "@/contentScript/partnerIntegrations";

registerMessenger();
registerExternalMessenger();
registerBuiltinBlocks();
registerContribBlocks();

function ignoreConnectionErrors(
function ignoreContextInvalidatedErrors(
errorEvent: ErrorEvent | PromiseRejectionEvent
): void {
if (isConnectionError(errorEvent)) {
showConnectionLost();
if (isContextInvalidatedError(errorEvent)) {
notifyContextInvalidated();
errorEvent.preventDefault();
}
}

// Must come before the default handler for ignoring errors. Otherwise, this handler might not be run
uncaughtErrorHandlers.unshift(ignoreConnectionErrors);
uncaughtErrorHandlers.unshift(ignoreContextInvalidatedErrors);

declare global {
interface Window {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,20 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import notify, { hideNotification } from "@/utils/notify";
import { isContextInvalidatedError } from "@/errors/contextInvalidated";

const id = "connection-lost";
describe("isContextInvalidatedError", () => {
const invalidated = new Error("Extension context invalidated.");
const unrelated = new Error("The ticket was not invalidated");
test("base test", () => {
expect(isContextInvalidatedError(invalidated)).toBeTrue();
expect(
isContextInvalidatedError(new Error("Parent", { cause: invalidated }))
).toBeTrue();

export function showConnectionLost(): void {
notify.error({
id,
message: "Connection to PixieBrix lost. Please reload the page",
duration: Number.POSITIVE_INFINITY,
expect(isContextInvalidatedError(unrelated)).toBeFalse();
expect(
isContextInvalidatedError(new Error("Parent", { cause: unrelated }))
).toBeFalse();
});
}

export function hideConnectionLost(): void {
hideNotification(id);
}
});
58 changes: 58 additions & 0 deletions src/errors/contextInvalidated.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* Copyright (C) 2022 PixieBrix, Inc.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import { expectContext } from "@/utils/expectContext";
import notify from "@/utils/notify";
import { once } from "lodash";
import {
CONTEXT_INVALIDATED_ERROR,
getErrorMessage,
getRootCause,
} from "./errorHelpers";

const id = "connection-lost";

export function notifyContextInvalidated(): void {
notify.error({
id,
message: "PixieBrix was updated or restarted. Reload the page to continue",
reportError: false,
duration: Number.POSITIVE_INFINITY,
});
}

/** Detects whether an error is a fatal context invalidation */
export function isContextInvalidatedError(possibleError: unknown): boolean {
return (
getErrorMessage(getRootCause(possibleError)) === CONTEXT_INVALIDATED_ERROR
);
}

export const wasContextInvalidated = () => !chrome.runtime?.id;

export const onContextInvalidated = once(async (): Promise<void> => {
expectContext("extension");

return new Promise((resolve) => {
const interval = setInterval(() => {
if (wasContextInvalidated()) {
resolve();
clearInterval(interval);
}
}, 200);
});
});
42 changes: 21 additions & 21 deletions src/errors/errorHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
*/

import { deserializeError, ErrorObject } from "serialize-error";
import { isObject, matchesAnyPattern, smartAppendPeriod } from "@/utils";
import { isObject, smartAppendPeriod } from "@/utils";
import safeJsonStringify from "json-stringify-safe";
import { truncate } from "lodash";
import type { ContextError } from "@/errors/genericErrors";
Expand All @@ -37,13 +37,11 @@ const DEFAULT_ERROR_MESSAGE = "Unknown error";
export const JQUERY_INVALID_SELECTOR_ERROR =
"Syntax error, unrecognized expression: ";

export const NO_TARGET_FOUND_CONNECTION_ERROR =
"Could not establish connection. Receiving end does not exist.";
/** Browser Messenger API error message patterns */
export const CONNECTION_ERROR_MESSAGES = [
NO_TARGET_FOUND_CONNECTION_ERROR,
"Extension context invalidated.",
];
/**
* Some APIs like runtime.sendMessage() and storage.get() will throw this error
* when the background page has been reloaded
*/
export const CONTEXT_INVALIDATED_ERROR = "Extension context invalidated.";

/**
* Errors to ignore unless they've caused extension point install or brick execution to fail.
Expand All @@ -59,13 +57,14 @@ export const IGNORED_ERROR_PATTERNS = [
"ResizeObserver loop limit exceeded",
"Promise was cancelled",
"Uncaught Error: PixieBrix contentScript already installed",
"Could not establish connection. Receiving end does not exist.",
"The frame was removed.",
/No frame with id \d+ in tab \d+/,
/^No tab with id/,
"The tab was closed.",
errorTabDoesntExist,
errorTargetClosedEarly,
...CONNECTION_ERROR_MESSAGES,
CONTEXT_INVALIDATED_ERROR,
];

export function isErrorObject(error: unknown): error is ErrorObject {
Expand Down Expand Up @@ -107,6 +106,19 @@ export function selectSpecificError<
return selectSpecificError(error.cause, errorType);
}

/**
* Follows the `Error#cause` trail until the end or returns the supplied object as is.
*
* @deprecated Prefer `hasSpecificErrorCause` or `getErrorMessageWithCauses` if you're displaying it to the user
*/
export function getRootCause(error: unknown): unknown {
while (isErrorObject(error) && error.cause) {
error = error.cause;
}

return error;
}

export function hasSpecificErrorCause<
ErrorType extends new (...args: unknown[]) => Error
>(error: unknown, errorType: ErrorType): boolean {
Expand Down Expand Up @@ -160,18 +172,6 @@ export function isClientRequestError(error: unknown): boolean {
return isErrorObject(error) && CLIENT_REQUEST_ERROR_NAMES.has(error.name);
}

/**
* Return true if the proximate cause of event is a messaging error.
*
* NOTE: does not recursively identify the root cause of the error.
*/
export function isConnectionError(possibleError: unknown): boolean {
return matchesAnyPattern(
getErrorMessage(possibleError),
CONNECTION_ERROR_MESSAGES
);
}

/**
* Return an error message corresponding to an error.
*/
Expand Down
2 changes: 1 addition & 1 deletion src/pageEditor/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { ensureContentScript } from "@/background/messenger/api";
import { canAccessTab } from "webext-tools";
import { sleep } from "@/utils";
import { useAsyncState } from "@/hooks/common";
import { onContextInvalidated } from "@/chrome";
import { onContextInvalidated } from "@/errors/contextInvalidated";

interface FrameMeta {
frameworks: FrameworkMeta[];
Expand Down
12 changes: 7 additions & 5 deletions src/telemetry/BackgroundLogger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@

import { Logger, MessageContext } from "@/core";
import { JsonObject } from "type-fest";
import { isConnectionError } from "@/errors/errorHelpers";
import { isContentScript } from "webext-detect-page";
import { showConnectionLost } from "@/contentScript/connection";
import { isBackground, isDevToolsPage } from "webext-detect-page";
import {
notifyContextInvalidated,
wasContextInvalidated,
} from "@/errors/contextInvalidated";
import { recordLog } from "@/background/messenger/api";
import { expectContext } from "@/utils/expectContext";
import reportError from "@/telemetry/reportError";
Expand Down Expand Up @@ -71,8 +73,8 @@ class BackgroundLogger implements Logger {
}

async error(error: unknown, data: JsonObject): Promise<void> {
if (isConnectionError(error) && isContentScript()) {
showConnectionLost();
if (wasContextInvalidated() && !isBackground() && !isDevToolsPage()) {
notifyContextInvalidated();
}

console.error("BackgroundLogger:error", {
Expand Down

0 comments on commit 3c62042

Please sign in to comment.