Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Make more of the codebase conform to strict types #10857

Merged
merged 7 commits into from
May 16, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 2 additions & 2 deletions src/ContentMessages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -580,13 +580,13 @@ export default class ContentMessages {
} catch (error) {
// 413: File was too big or upset the server in some way:
// clear the media size limit so we fetch it again next time we try to upload
if (error?.httpStatus === 413) {
if (error instanceof HTTPError && error.httpStatus === 413) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I just love typeguards too much, but I feel that a handful of isHTTPError()/isMatrixError() typeguards could be helpful due to how much they'd get used. What do you think?

Copy link
Member Author

@t3chguy t3chguy May 16, 2023

Choose a reason for hiding this comment

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

I agree for interfaces, but i prefer the built-in instanceof personally where possible, a typeguard leaves ambiguity on the table in the case of inheritence. If I pass a MatrixError to isHTTPError will it return true or false? I'd need to consult the docs, whereas I know instanceof will because MatrixError extends HTTPError

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah that's an interesting point. I think I generally use the typeguards for simpler cases and hadn't considered inheritance.

this.mediaConfig = null;
}

if (!upload.cancelled) {
let desc = _t("The file '%(fileName)s' failed to upload.", { fileName: upload.fileName });
if (error.httpStatus === 413) {
if (error instanceof HTTPError && error.httpStatus === 413) {
desc = _t("The file '%(fileName)s' exceeds this homeserver's size limit for uploads", {
fileName: upload.fileName,
});
Expand Down
4 changes: 2 additions & 2 deletions src/IdentityAuthClient.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ limitations under the License.

import React from "react";
import { SERVICE_TYPES } from "matrix-js-sdk/src/service-types";
import { createClient, MatrixClient } from "matrix-js-sdk/src/matrix";
import { createClient, MatrixClient, MatrixError } from "matrix-js-sdk/src/matrix";
import { logger } from "matrix-js-sdk/src/logger";

import { MatrixClientPeg } from "./MatrixClientPeg";
Expand Down Expand Up @@ -123,7 +123,7 @@ export default class IdentityAuthClient {
try {
await this.matrixClient.getIdentityAccount(token);
} catch (e) {
if (e.errcode === "M_TERMS_NOT_SIGNED") {
if (e instanceof MatrixError && e.errcode === "M_TERMS_NOT_SIGNED") {
logger.log("Identity server requires new terms to be agreed to");
await startTermsFlow([new Service(SERVICE_TYPES.IS, identityServerUrl, token)]);
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import FileSaver from "file-saver";
import { logger } from "matrix-js-sdk/src/logger";
import { IKeyBackupInfo } from "matrix-js-sdk/src/crypto/keybackup";
import { TrustInfo } from "matrix-js-sdk/src/crypto/backup";
import { CrossSigningKeys, UIAFlow } from "matrix-js-sdk/src/matrix";
import { CrossSigningKeys, MatrixError, UIAFlow } from "matrix-js-sdk/src/matrix";
import { IRecoveryKey } from "matrix-js-sdk/src/crypto/api";
import { CryptoEvent } from "matrix-js-sdk/src/crypto";
import classNames from "classnames";
Expand Down Expand Up @@ -208,7 +208,7 @@ export default class CreateSecretStorageDialog extends React.PureComponent<IProp
// no keys which would be a no-op.
logger.log("uploadDeviceSigningKeys unexpectedly succeeded without UI auth!");
} catch (error) {
if (!error.data || !error.data.flows) {
if (!(error instanceof MatrixError) || !error.data || !error.data.flows) {
logger.log("uploadDeviceSigningKeys advertised no flows!");
return;
}
Expand Down Expand Up @@ -372,7 +372,12 @@ export default class CreateSecretStorageDialog extends React.PureComponent<IProp
phase: Phase.Stored,
});
} catch (e) {
if (this.state.canUploadKeysWithPasswordOnly && e.httpStatus === 401 && e.data.flows) {
if (
this.state.canUploadKeysWithPasswordOnly &&
e instanceof MatrixError &&
e.httpStatus === 401 &&
e.data.flows
) {
this.setState({
accountPassword: "",
accountPasswordCorrect: false,
Expand Down
3 changes: 2 additions & 1 deletion src/boundThreepids.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ limitations under the License.

import { IThreepid, ThreepidMedium } from "matrix-js-sdk/src/@types/threepids";
import { MatrixClient } from "matrix-js-sdk/src/client";
import { MatrixError } from "matrix-js-sdk/src/http-api";

import IdentityAuthClient from "./IdentityAuthClient";

Expand Down Expand Up @@ -57,7 +58,7 @@ export async function getThreepidsWithBindStatus(
}
} catch (e) {
// Ignore terms errors here and assume other flows handle this
if (e.errcode !== "M_TERMS_NOT_SIGNED") {
if (!(e instanceof MatrixError) || e.errcode !== "M_TERMS_NOT_SIGNED") {
throw e;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/components/structures/auth/Login.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import classNames from "classnames";
import { logger } from "matrix-js-sdk/src/logger";
import { ISSOFlow, LoginFlow, SSOAction } from "matrix-js-sdk/src/@types/auth";

import { _t, _td } from "../../../languageHandler";
import { _t, _td, UserFriendlyError } from "../../../languageHandler";
import Login from "../../../Login";
import { messageForConnectionError, messageForLoginError } from "../../../utils/ErrorUtils";
import AutoDiscoveryUtils from "../../../utils/AutoDiscoveryUtils";
Expand Down Expand Up @@ -265,7 +265,7 @@ export default class LoginComponent extends React.PureComponent<IProps, IState>
logger.error("Problem parsing URL or unhandled error doing .well-known discovery:", e);

let message = _t("Failed to perform homeserver discovery");
if (e.translatedMessage) {
if (e instanceof UserFriendlyError && e.translatedMessage) {
message = e.translatedMessage;
}

Expand Down
7 changes: 6 additions & 1 deletion src/components/structures/auth/SoftLogout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import React, { ChangeEvent, SyntheticEvent } from "react";
import { logger } from "matrix-js-sdk/src/logger";
import { Optional } from "matrix-events-sdk";
import { ISSOFlow, LoginFlow, SSOAction } from "matrix-js-sdk/src/@types/auth";
import { MatrixError } from "matrix-js-sdk/src/http-api";

import { _t } from "../../../languageHandler";
import dis from "../../../dispatcher/dispatcher";
Expand Down Expand Up @@ -164,7 +165,11 @@ export default class SoftLogout extends React.Component<IProps, IState> {
credentials = await sendLoginRequest(hsUrl, isUrl, loginType, loginParams);
} catch (e) {
let errorText = _t("Failed to re-authenticate due to a homeserver problem");
if (e.errcode === "M_FORBIDDEN" && (e.httpStatus === 401 || e.httpStatus === 403)) {
if (
e instanceof MatrixError &&
e.errcode === "M_FORBIDDEN" &&
(e.httpStatus === 401 || e.httpStatus === 403)
) {
errorText = _t("Incorrect password");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export default class MessageEditHistoryDialog extends React.PureComponent<IProps
result = await client.relations(roomId, eventId, RelationType.Replace, EventType.RoomMessage, opts);
} catch (error) {
// log if the server returned an error
if (error.errcode) {
if (error instanceof MatrixError && error.errcode) {
logger.error("fetching /relations failed with error", error);
}
this.setState({ error: error as MatrixError }, () => reject(error));
Expand Down
4 changes: 2 additions & 2 deletions src/components/views/dialogs/ServerPickerDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { logger } from "matrix-js-sdk/src/logger";

import AutoDiscoveryUtils from "../../../utils/AutoDiscoveryUtils";
import BaseDialog from "./BaseDialog";
import { _t } from "../../../languageHandler";
import { _t, UserFriendlyError } from "../../../languageHandler";
import AccessibleButton from "../elements/AccessibleButton";
import SdkConfig from "../../../SdkConfig";
import Field from "../elements/Field";
Expand Down Expand Up @@ -113,7 +113,7 @@ export default class ServerPickerDialog extends React.PureComponent<IProps, ISta
const stateForError = AutoDiscoveryUtils.authComponentStateForError(e);
if (stateForError.serverErrorIsFatal) {
let error = _t("Unable to validate homeserver");
if (e.translatedMessage) {
if (e instanceof UserFriendlyError && e.translatedMessage) {
error = e.translatedMessage;
}
return { error };
Expand Down
3 changes: 2 additions & 1 deletion src/indexing/EventIndex.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { logger } from "matrix-js-sdk/src/logger";
import { EventType } from "matrix-js-sdk/src/@types/event";
import { ClientEvent, MatrixClient } from "matrix-js-sdk/src/client";
import { ISyncStateData, SyncState } from "matrix-js-sdk/src/sync";
import { HTTPError } from "matrix-js-sdk/src/http-api";

import PlatformPeg from "../PlatformPeg";
import { MatrixClientPeg } from "../MatrixClientPeg";
Expand Down Expand Up @@ -471,7 +472,7 @@ export default class EventIndex extends EventEmitter {
checkpoint.direction,
);
} catch (e) {
if (e.httpStatus === 403) {
if (e instanceof HTTPError && e.httpStatus === 403) {
logger.log(
"EventIndex: Removing checkpoint as we don't have ",
"permissions to fetch messages from this room.",
Expand Down
3 changes: 2 additions & 1 deletion src/utils/IdentityServerUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ limitations under the License.

import { SERVICE_TYPES } from "matrix-js-sdk/src/service-types";
import { logger } from "matrix-js-sdk/src/logger";
import { HTTPError } from "matrix-js-sdk/src/http-api";

import SdkConfig from "../SdkConfig";
import { MatrixClientPeg } from "../MatrixClientPeg";
Expand All @@ -39,7 +40,7 @@ export async function doesIdentityServerHaveTerms(fullUrl: string): Promise<bool
terms = await MatrixClientPeg.get().getTerms(SERVICE_TYPES.IS, fullUrl);
} catch (e) {
logger.error(e);
if (e.cors === "rejected" || e.httpStatus === 404) {
if (e.cors === "rejected" || (e instanceof HTTPError && e.httpStatus === 404)) {
terms = null;
} else {
throw e;
Expand Down