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

Replace newTranslatableError with UserFriendlyError (#10440 #10440

Merged
merged 17 commits into from
Mar 31, 2023
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
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
"matrix_lib_main": "./lib/index.ts",
"matrix_lib_typings": "./lib/index.d.ts",
"matrix_i18n_extra_translation_funcs": [
"newTranslatableError"
"UserFriendlyError"
MadLittleMods marked this conversation as resolved.
Show resolved Hide resolved
],
"scripts": {
"prepublishOnly": "yarn build",
Expand Down Expand Up @@ -203,7 +203,7 @@
"jest-mock": "^29.2.2",
"jest-raw-loader": "^1.0.1",
"matrix-mock-request": "^2.5.0",
"matrix-web-i18n": "^1.3.0",
"matrix-web-i18n": "^1.4.0",
"mocha-junit-reporter": "^2.2.0",
"node-fetch": "2",
"postcss-scss": "^4.0.4",
Expand Down
21 changes: 21 additions & 0 deletions src/@types/global.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,11 @@ declare global {
}

interface Error {
// Standard
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/cause
cause?: unknown;

// Non-standard
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/fileName
fileName?: string;
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/lineNumber
Expand All @@ -195,6 +200,22 @@ declare global {
columnNumber?: number;
}

// We can remove these pieces if we ever update to `target: "es2022"` in our
// TypeScript config which supports the new `cause` property, see
// https://github.com/vector-im/element-web/issues/24913
interface ErrorOptions {
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/cause
cause?: unknown;
}

interface ErrorConstructor {
new (message?: string, options?: ErrorOptions): Error;
(message?: string, options?: ErrorOptions): Error;
}

// eslint-disable-next-line no-var
var Error: ErrorConstructor;

// https://github.com/microsoft/TypeScript/issues/28308#issuecomment-650802278
interface AudioWorkletProcessor {
readonly port: MessagePort;
Expand Down
94 changes: 61 additions & 33 deletions src/SlashCommands.tsx

Large diffs are not rendered by default.

12 changes: 10 additions & 2 deletions src/components/views/right_panel/UserInfo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import { DeviceInfo } from "matrix-js-sdk/src/crypto/deviceinfo";

import dis from "../../../dispatcher/dispatcher";
import Modal from "../../../Modal";
import { _t } from "../../../languageHandler";
import { _t, UserFriendlyError } from "../../../languageHandler";
import DMRoomMap from "../../../utils/DMRoomMap";
import AccessibleButton, { ButtonEvent } from "../elements/AccessibleButton";
import SdkConfig from "../../../SdkConfig";
Expand Down Expand Up @@ -448,7 +448,15 @@ export const UserOptionsSection: React.FC<{
const inviter = new MultiInviter(roomId || "");
await inviter.invite([member.userId]).then(() => {
if (inviter.getCompletionState(member.userId) !== "invited") {
throw new Error(inviter.getErrorText(member.userId) ?? undefined);
const errorStringFromInviterUtility = inviter.getErrorText(member.userId);
if (errorStringFromInviterUtility) {
throw new Error(errorStringFromInviterUtility);
} else {
throw new UserFriendlyError(
`User (%(user)s) did not end up as invited to %(roomId)s but no error was given from the inviter utility`,
{ user: member.userId, roomId, cause: undefined },
);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The remaining TS strict errors are unrelated and tracked by element-hq/element-web#24899

And there are some nits from files we didn't even change

}
});
} catch (err) {
Expand Down
9 changes: 4 additions & 5 deletions src/editor/commands.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { IContent } from "matrix-js-sdk/src/models/event";
import EditorModel from "./model";
import { Type } from "./parts";
import { Command, CommandCategories, getCommand } from "../SlashCommands";
import { ITranslatableError, _t, _td } from "../languageHandler";
import { UserFriendlyError, _t, _td } from "../languageHandler";
import Modal from "../Modal";
import ErrorDialog from "../components/views/dialogs/ErrorDialog";
import QuestionDialog from "../components/views/dialogs/QuestionDialog";
Expand Down Expand Up @@ -65,7 +65,7 @@ export async function runSlashCommand(
): Promise<[content: IContent | null, success: boolean]> {
const result = cmd.run(roomId, threadId, args);
let messageContent: IContent | null = null;
let error = result.error;
let error: any = result.error;
if (result.promise) {
try {
if (cmd.category === CommandCategories.messages || cmd.category === CommandCategories.effects) {
Expand All @@ -86,9 +86,8 @@ export async function runSlashCommand(
let errText;
if (typeof error === "string") {
errText = error;
} else if ((error as ITranslatableError).translatedMessage) {
// Check for translatable errors (newTranslatableError)
errText = (error as ITranslatableError).translatedMessage;
} else if (error instanceof UserFriendlyError) {
errText = error.translatedMessage;
} else if (error.message) {
errText = error.message;
} else {
Expand Down
1 change: 1 addition & 0 deletions src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,7 @@
"Use an identity server to invite by email. Click continue to use the default identity server (%(defaultIdentityServerName)s) or manage in Settings.": "Use an identity server to invite by email. Click continue to use the default identity server (%(defaultIdentityServerName)s) or manage in Settings.",
"Continue": "Continue",
"Use an identity server to invite by email. Manage in Settings.": "Use an identity server to invite by email. Manage in Settings.",
"User (%(user)s) did not end up as invited to %(roomId)s but no error was given from the inviter utility": "User (%(user)s) did not end up as invited to %(roomId)s but no error was given from the inviter utility",
"Joins room with given address": "Joins room with given address",
"Leave room": "Leave room",
"Unrecognised room address: %(roomAlias)s": "Unrecognised room address: %(roomAlias)s",
Expand Down
76 changes: 57 additions & 19 deletions src/languageHandler.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,21 +46,49 @@ counterpart.setSeparator("|");
const FALLBACK_LOCALE = "en";
counterpart.setFallbackLocale(FALLBACK_LOCALE);

export interface ITranslatableError extends Error {
translatedMessage: string;
interface ErrorOptions {
// Because we're mixing the subsitution variables and `cause` into the same object
// below, we want them to always explicitly say whether there is an underlying error
// or not to avoid typos of "cause" slipping through unnoticed.
cause: unknown | undefined;
}

/**
* Helper function to create an error which has an English message
* with a translatedMessage property for use by the consumer.
* @param {string} message Message to translate.
* @param {object} variables Variable substitutions, e.g { foo: 'bar' }
* @returns {Error} The constructed error.
* Used to rethrow an error with a user-friendly translatable message while maintaining
* access to that original underlying error. Downstream consumers can display the
* `translatedMessage` property in the UI and inspect the underlying error with the
* `cause` property.
*
* The error message will display as English in the console and logs so Element
* developers can easily understand the error and find the source in the code. It also
* helps tools like Sentry deduplicate the error, or just generally searching in
* rageshakes to find all instances regardless of the users locale.
*
* @param message - The untranslated error message text, e.g "Something went wrong with %(foo)s".
* @param substitutionVariablesAndCause - Variable substitutions for the translation and
* original cause of the error. If there is no cause, just pass `undefined`, e.g { foo:
* 'bar', cause: err || undefined }
*/
export function newTranslatableError(message: string, variables?: IVariables): ITranslatableError {
const error = new Error(message) as ITranslatableError;
error.translatedMessage = _t(message, variables);
return error;
export class UserFriendlyError extends Error {
public readonly translatedMessage: string;

public constructor(message: string, substitutionVariablesAndCause?: IVariables & ErrorOptions) {
const errorOptions = {
cause: substitutionVariablesAndCause?.cause,
};
// Prevent "Could not find /%\(cause\)s/g in x" logs to the console by removing
// it from the list
const substitutionVariables = { ...substitutionVariablesAndCause };
delete substitutionVariables["cause"];

// Create the error with the English version of the message that we want to show
// up in the logs
const englishTranslatedMessage = _t(message, { ...substitutionVariables, locale: "en" });
super(englishTranslatedMessage, errorOptions);

// Also provide a translated version of the error in the users locale to display
this.translatedMessage = _t(message, substitutionVariables);
}
}

export function getUserLanguage(): string {
Expand Down Expand Up @@ -373,12 +401,18 @@ export function replaceByRegexes(text: string, mapping: IVariables | Tags): stri
}
}
if (!matchFoundSomewhere) {
// The current regexp did not match anything in the input
// Missing matches is entirely possible because you might choose to show some variables only in the case
// of e.g. plurals. It's still a bit suspicious, and could be due to an error, so log it.
// However, not showing count is so common that it's not worth logging. And other commonly unused variables
// here, if there are any.
if (regexpString !== "%\\(count\\)s") {
if (
// The current regexp did not match anything in the input. Missing
// matches is entirely possible because you might choose to show some
// variables only in the case of e.g. plurals. It's still a bit
// suspicious, and could be due to an error, so log it. However, not
// showing count is so common that it's not worth logging. And other
// commonly unused variables here, if there are any.
regexpString !== "%\\(count\\)s" &&
// Ignore the `locale` option which can be used to override the locale
// in counterpart
regexpString !== "%\\(locale\\)s"
) {
logger.log(`Could not find ${regexp} in ${text}`);
}
}
Expand Down Expand Up @@ -652,7 +686,11 @@ function doRegisterTranslations(customTranslations: ICustomTranslations): void {
* This function should be called *after* registering other translations data to
* ensure it overrides strings properly.
*/
export async function registerCustomTranslations(): Promise<void> {
export async function registerCustomTranslations({
testOnlyIgnoreCustomTranslationsCache = false,
}: {
testOnlyIgnoreCustomTranslationsCache?: boolean;
} = {}): Promise<void> {
const moduleTranslations = ModuleRunner.instance.allTranslations;
doRegisterTranslations(moduleTranslations);

Expand All @@ -661,7 +699,7 @@ export async function registerCustomTranslations(): Promise<void> {

try {
let json: Optional<ICustomTranslations>;
if (Date.now() >= cachedCustomTranslationsExpire) {
if (testOnlyIgnoreCustomTranslationsCache || Date.now() >= cachedCustomTranslationsExpire) {
json = CustomTranslationOptions.lookupFn
? CustomTranslationOptions.lookupFn(lookupUrl)
: ((await (await fetch(lookupUrl)).json()) as ICustomTranslations);
Expand Down
16 changes: 8 additions & 8 deletions src/utils/AutoDiscoveryUtils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { AutoDiscovery, ClientConfig } from "matrix-js-sdk/src/autodiscovery";
import { logger } from "matrix-js-sdk/src/logger";
import { IClientWellKnown } from "matrix-js-sdk/src/matrix";

import { _t, _td, newTranslatableError } from "../languageHandler";
import { _t, UserFriendlyError } from "../languageHandler";
import { makeType } from "./TypeUtils";
import SdkConfig from "../SdkConfig";
import { ValidatedServerConfig } from "./ValidatedServerConfig";
Expand Down Expand Up @@ -147,7 +147,7 @@ export default class AutoDiscoveryUtils {
syntaxOnly = false,
): Promise<ValidatedServerConfig> {
if (!homeserverUrl) {
throw newTranslatableError(_td("No homeserver URL provided"));
throw new UserFriendlyError("No homeserver URL provided");
}

const wellknownConfig: IClientWellKnown = {
Expand Down Expand Up @@ -199,7 +199,7 @@ export default class AutoDiscoveryUtils {
// This shouldn't happen without major misconfiguration, so we'll log a bit of information
// in the log so we can find this bit of codee but otherwise tell teh user "it broke".
logger.error("Ended up in a state of not knowing which homeserver to connect to.");
throw newTranslatableError(_td("Unexpected error resolving homeserver configuration"));
throw new UserFriendlyError("Unexpected error resolving homeserver configuration");
}

const hsResult = discoveryResult["m.homeserver"];
Expand All @@ -221,9 +221,9 @@ export default class AutoDiscoveryUtils {
logger.error("Error determining preferred identity server URL:", isResult);
if (isResult.state === AutoDiscovery.FAIL_ERROR) {
if (AutoDiscovery.ALL_ERRORS.indexOf(isResult.error as string) !== -1) {
throw newTranslatableError(isResult.error as string);
throw new UserFriendlyError(String(isResult.error));
}
throw newTranslatableError(_td("Unexpected error resolving identity server configuration"));
throw new UserFriendlyError("Unexpected error resolving identity server configuration");
} // else the error is not related to syntax - continue anyways.

// rewrite homeserver error since we don't care about problems
Expand All @@ -237,9 +237,9 @@ export default class AutoDiscoveryUtils {
logger.error("Error processing homeserver config:", hsResult);
if (!syntaxOnly || !AutoDiscoveryUtils.isLivelinessError(hsResult.error)) {
if (AutoDiscovery.ALL_ERRORS.indexOf(hsResult.error as string) !== -1) {
throw newTranslatableError(hsResult.error as string);
throw new UserFriendlyError(String(hsResult.error));
}
throw newTranslatableError(_td("Unexpected error resolving homeserver configuration"));
throw new UserFriendlyError("Unexpected error resolving homeserver configuration");
} // else the error is not related to syntax - continue anyways.
}

Expand All @@ -252,7 +252,7 @@ export default class AutoDiscoveryUtils {
// It should have been set by now, so check it
if (!preferredHomeserverName) {
logger.error("Failed to parse homeserver name from homeserver URL");
throw newTranslatableError(_td("Unexpected error resolving homeserver configuration"));
throw new UserFriendlyError("Unexpected error resolving homeserver configuration");
}

return makeType(ValidatedServerConfig, {
Expand Down
Loading