Only retry some webext-messenger errors

[fregante]

I think only specific errors should cause retries, and I think those are "target not found". Anything else should likely not be retried.

For example for https://github.com/pixiebrix/pixiebrix-source/issues/1677 I see the messenger retrying even though the error is expected to always fail.

Uncaught MessengerError: Conflict: The message GET_MANIFEST was handled by a third-party listener
    at pRetry.minTimeout (<anonymous>:958:16)
    at async RetryOperation._fn (<anonymous>:588:21)

[fregante]

I just found out that sidebar pings are not being retried. The point of the messenger is to simplify messaging and retries are part of that. Using pRetry around messenger methods defeats its purpose.

The retry logic is so hard to follow due to p-retry and its many try/catch requirements, so I'll drop the dependency and make the code more linear. It's 137 lines.

webext-messenger/source/sender.ts

Lines 90 to 227 in 53b0738

	// TODO: Split this up a bit because it's too long. Probably drop p-retry
	const response = await pRetry(
	async (attemptCount) => {
	const response = await sendMessage(attemptCount);
	if (isMessengerResponse(response)) {
	return response;
	}
	// If no one answers, `response` will be `undefined`
	// If the target does not have any `onMessage` listener at all, it will throw
	// Possible:
	// - Any target exists and has onMessage handler, but never handled the message
	// - Extension page exists and has Messenger, but never handled the message (Messenger in Runtime ignores messages when the target isn't found)
	// Not possible:
	// - Tab exists and has Messenger, but never handled the message (Messenger in CS always handles messages)
	// - Any target exists, but Messenger didn't have the specific Type handler (The receiving Messenger will throw an error)
	// - No targets exist (the browser immediately throws "Could not establish connection. Receiving end does not exist.")
	if (response === undefined) {
	if ("page" in target) {
	throw new MessengerError(
	`The target ${JSON.stringify(target)} for ${type} was not found`,
	);
	}
	throw new MessengerError(
	`Messenger was not available in the target ${JSON.stringify(
	target,
	)} for ${type}`,
	);
	}
	// Possible:
	// - Non-Messenger handler responded
	throw new MessengerError(
	`Conflict: The message ${type} was handled by a third-party listener`,
	);
	},
	{
	minTimeout: 100,
	factor: 1.3,
	// Do not set this to undefined or Infinity, it doesn't work the same way
	...(retry ? {} : { retries: 0 }),
	maxRetryTime: 4000,
	async onFailedAttempt(error) {
	events.dispatchEvent(
	new CustomEvent("failed-attempt", {
	detail: {
	type,
	seq,
	target,
	error,
	attemptCount: error.attemptNumber,
	},
	}),
	);
	if (
	"extensionId" in target &&
	error.message === _errorNonExistingTarget
	) {
	// The extension is not available and it will not be. Do not retry.
	throw new ExtensionNotFoundError(
	errorExtensionNotFound.replace("$ID", target.extensionId!),
	);
	}
	if (isExtensionContext() && wasContextInvalidated()) {
	// The error matches the native context invalidated error
	// *.sendMessage() might fail with a message-specific error that is less useful,
	// like "Sender closed without responding"
	throw new Error("Extension context invalidated.");
	}
	if (error.message === _errorTargetClosedEarly) {
	throw new Error(errorTargetClosedEarly);
	}
	if (
	!(
	// If NONE of these conditions is true, stop retrying
	// Don't retry sending to the background page unless it really hasn't loaded yet
	(
	(target.page !== "background" &&
	error instanceof MessengerError) ||
	// Page or its content script not yet loaded
	error.message === _errorNonExistingTarget ||
	// `registerMethods` not yet loaded
	String(error.message).startsWith("No handlers registered in ")
	)
	)
	) {
	throw error;
	}
	if (chrome.tabs && typeof target.tabId === "number") {
	try {
	const tabInfo = await chrome.tabs.get(target.tabId);
	if (tabInfo.discarded) {
	throw new Error(errorTabWasDiscarded);
	}
	} catch {
	throw new Error(errorTabDoesntExist);
	}
	}
	log.debug(type, seq, "will retry. Attempt", error.attemptNumber);
	},
	},
	).catch((error: unknown) => {
	if (
	error &&
	typeof error === "object" &&
	"message" in error &&
	error?.message === _errorNonExistingTarget
	) {
	throw new MessengerError(
	`The target ${JSON.stringify(target)} for ${type} was not found`,
	);
	}
	events.dispatchEvent(
	new CustomEvent("attempts-exhausted", {
	detail: { type, seq, target, error },
	}),
	);
	throw error;
	});
	if ("error" in response) {
	log.debug(type, seq, "↘️ replied with error", response.error);
	throw deserializeError(response.error);
	}
	log.debug(type, seq, "↘️ replied successfully", response.value);
	return response.value;
	}

This was noticed when looking into:

• https://github.com/pixiebrix/pixiebrix-source/issues/1838

[fregante]

I just found out that sidebar pings are not being retried

This was a setup error in the extension. Fixed:

• https://github.com/pixiebrix/pixiebrix-source/pull/4675#discussion_r2454465781

The retry logic is so hard to follow due to p-retry

Done:

• Drop p-retry; fix retry bugs #362

Only retry some webext-messenger errors

Not done yet, but after my last PR we have a simple shouldRetryError function we can customize.