Skip to content
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.

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

Continue fixing useUnknownInCatchVariables #2054

Merged
merged 2 commits into from
Dec 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
"unicorn/no-await-expression-member": "warn", // Annoying sometimes, let's try it

"@typescript-eslint/consistent-type-assertions": "warn",
"@typescript-eslint/no-implicit-any-catch": "warn",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This rule isn't needed when useUnknownInCatchVariables is active


"jsx-a11y/click-events-have-key-events": "warn",
"jsx-a11y/no-static-element-interactions": "warn",
Expand Down
15 changes: 15 additions & 0 deletions src/Globals.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,18 @@ interface HTMLDialogElement extends HTMLElement {
declare const jsdom: {
reconfigure: (options: { url: string }) => void;
};

// `useUnknownInCatchVariables` for .catch method https://github.com/microsoft/TypeScript/issues/45602
interface Promise<T> {
/**
* Attaches a callback for only the rejection of the Promise.
* @param onrejected The callback to execute when the Promise is rejected.
* @returns A Promise for the completion of the callback.
*/
catch<TResult = never>(
onrejected?:
| ((reason: unknown) => TResult | PromiseLike<TResult>)
| undefined
| null
): Promise<T | TResult>;
}
2 changes: 1 addition & 1 deletion src/actionPanel/native.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export function showActionPanel(callbacks = extensionCallbacks): string {
for (const callback of callbacks) {
try {
callback();
} catch (error: unknown) {
} catch (error) {
// The callbacks should each have their own error handling. But wrap in a try-catch to ensure running
// the callbacks does not interfere prevent showing the sidebar
reportError(error);
Expand Down
2 changes: 1 addition & 1 deletion src/actionPanel/protocol.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ function runListeners<Method extends keyof StoreListener>(
// @ts-expect-error `data` is a intersection type instead of an union. TODO: Fix or rewrite
// eslint-disable-next-line security/detect-object-injection -- method is keyof StoreListener
listener[method](data);
} catch (error: unknown) {
} catch (error) {
reportError(error);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/background/__mocks__/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export function liftBackground<

try {
handlerResult = await method(...args);
} catch (error: unknown) {
} catch (error) {
console.log("Error running method", error);
handlerResult = toErrorResponse(fullType, error);
}
Expand Down
2 changes: 1 addition & 1 deletion src/background/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ async function codeGrantFlow(
"Content-Type": "application/x-www-form-urlencoded",
},
});
} catch (error: unknown) {
} catch (error) {
console.error(error);
throw new Error(`Error getting OAuth2 token: ${getErrorMessage(error)}`);
}
Expand Down
2 changes: 1 addition & 1 deletion src/background/browserAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ async function handleBrowserAction(tab: Tabs.Tab): Promise<void> {
tabId: tab.id,
frameId: TOP_LEVEL_FRAME_ID,
});
} catch (error: unknown) {
} catch (error) {
await showErrorInOptions("ERR_BROWSER_ACTION_TOGGLE", tab.index);
console.error(error);
reportError(error);
Expand Down
10 changes: 5 additions & 5 deletions src/background/contextMenus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ async function dispatchMenu(

try {
reportEvent("ContextMenuClick", { extensionId: info.menuItemId });
} catch (error: unknown) {
} catch (error) {
console.warn("Error reporting ContextMenuClick event", { error });
}

Expand All @@ -96,7 +96,7 @@ async function dispatchMenu(
message: "Ran content menu item action",
className: "success",
});
} catch (error: unknown) {
} catch (error) {
if (hasCancelRootCause(error)) {
void showNotification(target, {
message: "The action was cancelled",
Expand Down Expand Up @@ -145,7 +145,7 @@ export async function uninstallContextMenu({

console.debug(`Uninstalled context menu ${extensionId}`);
return true;
} catch (error: unknown) {
} catch (error) {
// Will throw if extensionId doesn't refer to a context menu. The callers don't have an easy way to check the type
// without having to resolve the extensionPointId. So instead we'll just expect some of the calls to fail.
console.debug("Could not uninstall context menu %s", extensionId, {
Expand Down Expand Up @@ -193,7 +193,7 @@ export async function ensureContextMenu({
try {
await browser.contextMenus.update(menuId, updateProperties);
return;
} catch (error: unknown) {
} catch (error) {
console.debug("Cannot update context menu", { error });
}
} else {
Expand Down Expand Up @@ -225,7 +225,7 @@ export async function ensureContextMenu({
});

extensionMenuItems.set(extensionId, menuId);
} catch (error: unknown) {
} catch (error) {
if (
getErrorMessage(error).includes("Cannot create item with duplicate id")
) {
Expand Down
4 changes: 2 additions & 2 deletions src/background/deployment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ async function updateDeployments() {
// Fetch the current brick definitions, which will have the current permissions and extensionVersion requirements
try {
await refreshRegistries();
} catch (error: unknown) {
} catch (error) {
reportError(error);
await browser.runtime.openOptionsPage();
// Bail and open the main options page, which 1) fetches the latest bricks, and 2) will prompt the user the to
Expand Down Expand Up @@ -237,7 +237,7 @@ async function updateDeployments() {
console.info(
`Applied automatic updates for ${automatic.length} deployment(s)`
);
} catch (error: unknown) {
} catch (error) {
reportError(error);
automaticError = error;
}
Expand Down
4 changes: 2 additions & 2 deletions src/background/devtools/external.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export async function callBackground(
if (isNotification(options)) {
try {
port.postMessage(message);
} catch (error: unknown) {
} catch (error) {
throw new Error(
`Error sending devtools notification: ${getErrorMessage(error)}`
);
Expand All @@ -110,7 +110,7 @@ export async function callBackground(
devtoolsHandlers.set(nonce, [resolve, reject]);
try {
port.postMessage(message);
} catch (error: unknown) {
} catch (error) {
reject(
new Error(`Error sending devtools message: ${getErrorMessage(error)}`)
);
Expand Down
6 changes: 3 additions & 3 deletions src/background/devtools/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ function backgroundMessageListener(
let responded = false;

if (notification) {
handlerPromise.catch((error: unknown) => {
handlerPromise.catch((error) => {
console.warn(
`An error occurred when handling notification ${type} (nonce: ${meta?.nonce})`,
error
Expand Down Expand Up @@ -185,7 +185,7 @@ export function liftBackground<
async function resetTab(tabId: number): Promise<void> {
try {
await clearDynamicElements({ tabId, frameId: TOP_LEVEL_FRAME_ID }, {});
} catch (error: unknown) {
} catch (error) {
console.warn("Error clearing dynamic elements for tab: %d", tabId, {
error,
});
Expand Down Expand Up @@ -280,7 +280,7 @@ async function attemptTemporaryAccess({

try {
await ensureContentScript({ tabId, frameId });
} catch (error: unknown) {
} catch (error) {
if (isPrivatePageError(error)) {
return;
}
Expand Down
2 changes: 1 addition & 1 deletion src/background/executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ async function retrySend<T extends (...args: unknown[]) => Promise<unknown>>(
try {
// eslint-disable-next-line no-await-in-loop -- retry loop
return await send();
} catch (error: unknown) {
} catch (error) {
const message = getErrorMessage(error);

if (NOT_READY_PARTIAL_MESSAGES.some((query) => message.includes(query))) {
Expand Down
6 changes: 3 additions & 3 deletions src/background/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ async function handleRequest(
`Handler FULFILLED action ${type} (nonce: ${meta?.nonce}, tab: ${sender.tab?.id}, frame: ${sender.frameId})`
);
return value;
} catch (error: unknown) {
} catch (error) {
if (isNotification(options)) {
console.warn(
`An error occurred when handling notification ${type} (nonce: ${meta?.nonce}, tab: ${sender.tab?.id}, frame: ${sender.frameId})`,
Expand Down Expand Up @@ -114,7 +114,7 @@ async function callBackground(
console.debug(`Sending background notification ${type} (nonce: ${nonce})`, {
extensionId,
});
sendMessage(extensionId, message, {}).catch((error: unknown) => {
sendMessage(extensionId, message, {}).catch((error) => {
console.warn(
`An error occurred processing background notification ${type} (nonce: ${nonce})`,
error
Expand All @@ -127,7 +127,7 @@ async function callBackground(
let response;
try {
response = await sendMessage(extensionId, message, {});
} catch (error: unknown) {
} catch (error) {
console.debug(
`Error sending background action ${type} (nonce: ${nonce})`,
{ extensionId, error }
Expand Down
8 changes: 4 additions & 4 deletions src/background/proxyService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ describe("unauthenticated direct requests", () => {
try {
await proxyService(null, requestConfig);
fail("Expected proxyService to throw a RemoteServiceError error");
} catch (error: unknown) {
} catch (error) {
expect(error).toBeInstanceOf(RemoteServiceError);
const { status } = (error as RemoteServiceError).response;
expect(status).toEqual(500);
Expand Down Expand Up @@ -149,7 +149,7 @@ describe("authenticated direct requests", () => {
try {
await proxyService(directServiceConfig, requestConfig);
fail("Expected proxyService to throw an error");
} catch (error: unknown) {
} catch (error) {
expect(error).toBeInstanceOf(ContextError);
const { status } = ((error as ContextError).cause as AxiosError).response;
expect(status).toEqual(403);
Expand Down Expand Up @@ -191,7 +191,7 @@ describe("proxy service requests", () => {
try {
await proxyService(proxiedServiceConfig, requestConfig);
fail("Expected proxyService to throw an error");
} catch (error: unknown) {
} catch (error) {
expect(error).toBeInstanceOf(ContextError);
const { status, statusText } = ((error as ContextError)
.cause as AxiosError).response;
Expand All @@ -208,7 +208,7 @@ describe("proxy service requests", () => {
try {
await proxyService(proxiedServiceConfig, requestConfig);
fail("Expected proxyService to throw an error");
} catch (error: unknown) {
} catch (error) {
expect(error).toBeInstanceOf(Error);
expect((error as Error).message).toEqual(
"API proxy error: Request failed with status code 500"
Expand Down
10 changes: 5 additions & 5 deletions src/background/requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ const backgroundRequest = liftBackground(
async (config: AxiosRequestConfig) => {
try {
return cleanResponse(await axios(config));
} catch (error: unknown) {
} catch (error) {
if (isAxiosError(error)) {
// Axios offers its own serialization method, but it doesn't include the response.
// By deleting toJSON, the serialize-error library will use its default serialization
Expand Down Expand Up @@ -212,7 +212,7 @@ async function proxyRequest<T>(
proxyResponse = (await backgroundRequest(
authenticatedRequestConfig
)) as SanitizedResponse<ProxyResponseData>;
} catch (error: unknown) {
} catch (error) {
// If there's a server error with the proxy server itself, we'll also see it in the Rollbar logs for the server.
throw new Error(`API proxy error: ${getErrorMessage(error)}`);
}
Expand Down Expand Up @@ -254,7 +254,7 @@ const _proxyService = liftBackground(
return await backgroundRequest(
await authenticate(serviceConfig, requestConfig)
);
} catch (error: unknown) {
} catch (error) {
// Try again - have the user login again, or automatically try to get a new token
if (
isAxiosError(error) &&
Expand Down Expand Up @@ -309,7 +309,7 @@ export async function proxyService<TData>(
return (await backgroundRequest(
requestConfig
)) as SanitizedResponse<TData>;
} catch (error: unknown) {
} catch (error) {
if (!isAxiosError(error)) {
throw error;
}
Expand All @@ -331,7 +331,7 @@ export async function proxyService<TData>(
serviceConfig,
requestConfig
)) as RemoteResponse<TData>;
} catch (error: unknown) {
} catch (error) {
throw new ContextError(selectError(error) as Error, {
serviceId: serviceConfig.serviceId,
authId: serviceConfig.id,
Expand Down
2 changes: 1 addition & 1 deletion src/background/telemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ async function userSummary() {
.length;
numActiveExtensionPoints = uniq(extensions.map((x) => x.extensionPointId))
.length;
} catch (error: unknown) {
} catch (error) {
console.warn("Cannot get number of extensions", { error });
}

Expand Down
2 changes: 1 addition & 1 deletion src/baseRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ export class Registry<
private parse(raw: unknown): Item | undefined {
try {
return this.deserialize(raw);
} catch (error: unknown) {
} catch (error) {
console.warn(
"Error de-serializing item: %s",
getErrorMessage(error),
Expand Down
2 changes: 1 addition & 1 deletion src/blocks/effects/wait.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export class WaitElementEffect extends Effect {
const [promise, cancel] = awaitElementOnce(selector);
try {
await runInMillis(async () => promise, maxWaitMillis);
} catch (error: unknown) {
} catch (error) {
cancel();

if (error instanceof TimeoutError) {
Expand Down
2 changes: 1 addition & 1 deletion src/blocks/renderers/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export async function errorBoundary(

// TODO: validate the shape of the value returned
return value;
} catch (error: unknown) {
} catch (error) {
logger.error(error);
const escapedMessage = escape(getErrorMessage(error));
return sanitize(`<div>An error occurred: ${escapedMessage}</div>`);
Expand Down
2 changes: 1 addition & 1 deletion src/blocks/renderers/customForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export class CustomFormRenderer extends Renderer {
className: "success",
},
});
} catch (error: unknown) {
} catch (error) {
reportError(error);
notifyResult(logger.context.extensionId, {
message: "Error saving record",
Expand Down
3 changes: 1 addition & 2 deletions src/blocks/transformers/ephemeralForm/formTransformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import {
hideActionPanelForm,
} from "@/actionPanel/native";
import { showModal } from "@/blocks/transformers/ephemeralForm/modalUtils";
import { unary } from "lodash";
import { reportError } from "@/telemetry/logging";
import pDefer from "p-defer";

Expand Down Expand Up @@ -160,7 +159,7 @@ export class FormTransformer extends Transformer {
// In the future we might creating/sending a closeIfEmpty message to the sidebar, so that it would close
// if this form was the only entry in the panel
hideActionPanelForm(nonce);
void cancelForm(nonce).catch(unary(reportError));
void cancelForm(nonce).catch(reportError);
});
} else {
showModal(frameSrc, controller.signal);
Expand Down
2 changes: 1 addition & 1 deletion src/blocks/transformers/jq.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export class JQTransformer extends Transformer {
try {
// eslint-disable-next-line @typescript-eslint/return-await -- Type is `any`, it throws the rule off
return await jq.promised.json(input, filter);
} catch (error: unknown) {
} catch (error) {
if (!isErrorObject(error) || !error.message.includes("compile error")) {
throw error;
}
Expand Down
2 changes: 1 addition & 1 deletion src/components/AsyncIcon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ async function handleIconImport(
try {
const { definition } = await moduleImport;
return definition;
} catch (error: unknown) {
} catch (error) {
console.warn("Error importing FontAwesome icon library module", { error });
throw error;
}
Expand Down
2 changes: 1 addition & 1 deletion src/components/ellipsisMenu/EllipsisMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ const EllipsisMenu: React.FunctionComponent<{
// hence no other element knows that the click happened.
// Simulating the click on the body will let other menus know user clicked somewhere.
document.body.click();
} catch (error: unknown) {
} catch (error) {
console.debug(
"EllipsisMenu. Failed to trigger closing other menus",
error
Expand Down
Loading