Improve rageshake upload experience by providing useful error information (#29378)

Half-Shot · web-flow · commit 3762d40620c6 · 2025-03-06T08:38:41.000Z
* Refactor submit rageshake so that it uses the new error codes. * Improve error information given in Bug Report Dialog * use type * Refactor with generic error & policy link. * lint * lint * Add BugReportDialog test * fix time travel * use waitFor while waiting for fetch to finish * lint * Drop error prefix as per matrix-org/rageshake@3973bb3 * small fixes * Don't change string here. * Fixup i18n strings.
diff --git a/src/components/views/dialogs/BugReportDialog.tsx b/src/components/views/dialogs/BugReportDialog.tsx
@@ -9,12 +9,13 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com
 Please see LICENSE files in the repository root for full details.
 */
 
-import React from "react";
+import React, { type ReactNode } from "react";
+import { Link } from "@vector-im/compound-web";
 
 import SdkConfig from "../../../SdkConfig";
 import Modal from "../../../Modal";
 import { _t } from "../../../languageHandler";
-import sendBugReport, { downloadBugReport } from "../../../rageshake/submit-rageshake";
+import sendBugReport, { downloadBugReport, RageshakeError } from "../../../rageshake/submit-rageshake";
 import AccessibleButton from "../elements/AccessibleButton";
 import QuestionDialog from "./QuestionDialog";
 import BaseDialog from "./BaseDialog";
@@ -26,7 +27,7 @@ import defaultDispatcher from "../../../dispatcher/dispatcher";
 import { Action } from "../../../dispatcher/actions";
 import { getBrowserSupport } from "../../../SupportedBrowser";
 
-interface IProps {
+export interface BugReportDialogProps {
     onFinished: (success: boolean) => void;
     initialText?: string;
     label?: string;
@@ -36,19 +37,19 @@ interface IProps {
 interface IState {
     sendLogs: boolean;
     busy: boolean;
-    err: string | null;
+    err: ReactNode | null;
     issueUrl: string;
     text: string;
     progress: string | null;
     downloadBusy: boolean;
     downloadProgress: string | null;
 }
 
-export default class BugReportDialog extends React.Component<IProps, IState> {
+export default class BugReportDialog extends React.Component<BugReportDialogProps, IState> {
     private unmounted: boolean;
     private issueRef: React.RefObject<Field>;
 
-    public constructor(props: IProps) {
+    public constructor(props: BugReportDialogProps) {
         super(props);
 
         this.state = {
@@ -89,6 +90,42 @@ export default class BugReportDialog extends React.Component<IProps, IState> {
         this.props.onFinished(false);
     };
 
+    private getErrorText(error: Error | RageshakeError): ReactNode {
+        if (error instanceof RageshakeError) {
+            let errorText;
+            switch (error.errorcode) {
+                case "DISALLOWED_APP":
+                    errorText = _t("bug_reporting|failed_send_logs_causes|disallowed_app");
+                    break;
+                case "REJECTED_BAD_VERSION":
+                    errorText = _t("bug_reporting|failed_send_logs_causes|rejected_version");
+                    break;
+                case "REJECTED_UNEXPECTED_RECOVERY_KEY":
+                    errorText = _t("bug_reporting|failed_send_logs_causes|rejected_recovery_key");
+                    break;
+                default:
+                    if (error.errorcode?.startsWith("REJECTED")) {
+                        errorText = _t("bug_reporting|failed_send_logs_causes|rejected_generic");
+                    } else {
+                        errorText = _t("bug_reporting|failed_send_logs_causes|server_unknown_error");
+                    }
+                    break;
+            }
+            return (
+                <>
+                    <p>{errorText}</p>
+                    {error.policyURL && (
+                        <Link size="medium" target="_blank" href={error.policyURL}>
+                            {_t("action|learn_more")}
+                        </Link>
+                    )}
+                </>
+            );
+        } else {
+            return <p>{_t("bug_reporting|failed_send_logs_causes|unknown_error")}</p>;
+        }
+    }
+
     private onSubmit = (): void => {
         if ((!this.state.text || !this.state.text.trim()) && (!this.state.issueUrl || !this.state.issueUrl.trim())) {
             this.setState({
@@ -126,7 +163,7 @@ export default class BugReportDialog extends React.Component<IProps, IState> {
                     this.setState({
                         busy: false,
                         progress: null,
-                        err: _t("bug_reporting|failed_send_logs") + `${err.message}`,
+                        err: this.getErrorText(err),
                     });
                 }
             },
@@ -155,7 +192,7 @@ export default class BugReportDialog extends React.Component<IProps, IState> {
                 this.setState({
                     downloadBusy: false,
                     downloadProgress:
-                        _t("bug_reporting|failed_send_logs") + `${err instanceof Error ? err.message : ""}`,
+                        _t("bug_reporting|failed_download_logs") + `${err instanceof Error ? err.message : ""}`,
                 });
             }
         }
diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json
@@ -407,7 +407,15 @@
         "download_logs": "Download logs",
         "downloading_logs": "Downloading logs",
         "error_empty": "Please tell us what went wrong or, better, create a GitHub issue that describes the problem.",
-        "failed_send_logs": "Failed to send logs: ",
+        "failed_download_logs": "Failed to download debug logs: ",
+        "failed_send_logs_causes": {
+            "disallowed_app": "Your bug report was rejected. The rageshake server does not support this application.",
+            "rejected_generic": "Your bug report was rejected. The rageshake server rejected the contents of the report due to a policy.",
+            "rejected_recovery_key": "Your bug report was rejected for safety reasons, as it contained a recovery key.",
+            "rejected_version": "Your bug report was rejected as the version you are running is too old.",
+            "server_unknown_error": "The rageshake server encountered an unknown error and could not handle the report.",
+            "unknown_error": "Failed to send logs."
+        },
         "github_issue": "GitHub issue",
         "introduction": "If you've submitted a bug via GitHub, debug logs can help us track down the problem. ",
         "log_request": "To help us prevent this in future, please <a>send us logs</a>.",
diff --git a/src/rageshake/submit-rageshake.ts b/src/rageshake/submit-rageshake.ts
@@ -1,5 +1,5 @@
 /*
-Copyright 2024 New Vector Ltd.
+Copyright 2024, 2025 New Vector Ltd.
 Copyright 2019 The Matrix.org Foundation C.I.C.
 Copyright 2018 New Vector Ltd
 Copyright 2017 OpenMarket Ltd
@@ -30,6 +30,24 @@ interface IOpts {
     customFields?: Record<string, string>;
 }
 
+export class RageshakeError extends Error {
+    /**
+     * This error is thrown when the rageshake server cannot process the request.
+     * @param errorcode Machine-readable error code. See https://github.com/matrix-org/rageshake/blob/main/docs/api.md
+     * @param error A human-readable error.
+     * @param statusCode The HTTP status code.
+     * @param policyURL Optional policy URL that can be presented to the user.
+     */
+    public constructor(
+        public readonly errorcode: string,
+        public readonly error: string,
+        public readonly statusCode: number,
+        public readonly policyURL?: string,
+    ) {
+        super(`The rageshake server responded with an error ${errorcode} (${statusCode}): ${error}`);
+    }
+}
+
 /**
  * Exported only for testing.
  * @internal public for test
@@ -323,6 +341,9 @@ async function collectLogs(
  * @param {function(string)} opts.progressCallback Callback to call with progress updates
  *
  * @return {Promise<string>} URL returned by the rageshake server
+ *
+ * @throws A RageshakeError when the rageshake server responds with an error. This will be `RS_UNKNOWN` if the
+ *         the server does not respond with an expected body format.
  */
 export default async function sendBugReport(bugReportEndpoint?: string, opts: IOpts = {}): Promise<string> {
     if (!bugReportEndpoint) {
@@ -426,24 +447,37 @@ export async function submitFeedback(
     }
 }
 
-function submitReport(endpoint: string, body: FormData, progressCallback: (str: string) => void): Promise<string> {
-    return new Promise<string>((resolve, reject) => {
-        const req = new XMLHttpRequest();
-        req.open("POST", endpoint);
-        req.responseType = "json";
-        req.timeout = 5 * 60 * 1000;
-        req.onreadystatechange = function (): void {
-            if (req.readyState === XMLHttpRequest.LOADING) {
-                progressCallback(_t("bug_reporting|waiting_for_server"));
-            } else if (req.readyState === XMLHttpRequest.DONE) {
-                // on done
-                if (req.status < 200 || req.status >= 400) {
-                    reject(new Error(`HTTP ${req.status}`));
-                    return;
-                }
-                resolve(req.response.report_url || "");
-            }
-        };
-        req.send(body);
+/**
+ * Submit a rageshake report to the rageshake server.
+ *
+ * @param endpoint The endpoint to call.
+ * @param body The report body.
+ * @param progressCallback A callback that will be called when the upload process has begun.
+ * @returns The URL to the public report.
+ * @throws A RageshakeError when the rageshake server responds with an error. This will be `RS_UNKNOWN` if the
+ *         the server does not respond with an expected body format.
+ */
+async function submitReport(
+    endpoint: string,
+    body: FormData,
+    progressCallback: (str: string) => void,
+): Promise<string> {
+    const req = fetch(endpoint, {
+        method: "POST",
+        body,
+        signal: AbortSignal.timeout?.(5 * 60 * 1000),
     });
+    progressCallback(_t("bug_reporting|waiting_for_server"));
+    const response = await req;
+    if (response.headers.get("Content-Type") !== "application/json") {
+        throw new RageshakeError("UNKNOWN", "Rageshake server responded with unexpected type", response.status);
+    }
+    const data = await response.json();
+    if (response.status < 200 || response.status >= 400) {
+        if ("errcode" in data) {
+            throw new RageshakeError(data.errcode, data.error, response.status, data.policy_url);
+        }
+        throw new RageshakeError("UNKNOWN", "Rageshake server responded with unexpected type", response.status);
+    }
+    return data.report_url;
 }
diff --git a/test/unit-tests/components/views/dialogs/BugReportDialog-test.tsx b/test/unit-tests/components/views/dialogs/BugReportDialog-test.tsx
@@ -0,0 +1,132 @@
+/*
+Copyright 2025 New Vector Ltd.
+
+SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
+Please see LICENSE files in the repository root for full details.
+*/
+
+import { render, waitFor, type RenderResult } from "jest-matrix-react";
+import userEvent from "@testing-library/user-event";
+import React from "react";
+import fetchMock from "fetch-mock-jest";
+import { type Mocked } from "jest-mock";
+
+import BugReportDialog, {
+    type BugReportDialogProps,
+} from "../../../../../src/components/views/dialogs/BugReportDialog";
+import SdkConfig from "../../../../../src/SdkConfig";
+import { type ConsoleLogger } from "../../../../../src/rageshake/rageshake";
+
+const BUG_REPORT_URL = "https://example.org/submit";
+
+describe("BugReportDialog", () => {
+    const onFinished: jest.Mock<any, any> = jest.fn();
+
+    function renderComponent(props: Partial<BugReportDialogProps> = {}): RenderResult {
+        return render(<BugReportDialog onFinished={onFinished} />);
+    }
+
+    beforeEach(() => {
+        jest.resetAllMocks();
+        SdkConfig.put({
+            bug_report_endpoint_url: BUG_REPORT_URL,
+        });
+
+        const mockConsoleLogger = {
+            flush: jest.fn(),
+            consume: jest.fn(),
+            warn: jest.fn(),
+        } as unknown as Mocked<ConsoleLogger>;
+
+        // @ts-ignore - mock the console logger
+        global.mx_rage_logger = mockConsoleLogger;
+
+        // @ts-ignore
+        mockConsoleLogger.flush.mockReturnValue([
+            {
+                id: "instance-0",
+                line: "line 1",
+            },
+            {
+                id: "instance-1",
+                line: "line 2",
+            },
+        ]);
+    });
+
+    afterEach(() => {
+        SdkConfig.reset();
+        fetchMock.restore();
+    });
+
+    it("can close the bug reporter", async () => {
+        const { getByTestId } = renderComponent();
+        await userEvent.click(getByTestId("dialog-cancel-button"));
+        expect(onFinished).toHaveBeenCalledWith(false);
+    });
+
+    it("can submit a bug report", async () => {
+        const { getByLabelText, getByText } = renderComponent();
+        fetchMock.postOnce(BUG_REPORT_URL, { report_url: "https://exmaple.org/report/url" });
+        await userEvent.type(getByLabelText("GitHub issue"), "https://example.org/some/issue");
+        await userEvent.type(getByLabelText("Notes"), "Additional text");
+        await userEvent.click(getByText("Send logs"));
+        await waitFor(() => expect(getByText("Thank you!")).toBeInTheDocument());
+        expect(onFinished).toHaveBeenCalledWith(false);
+        expect(fetchMock).toHaveFetched(BUG_REPORT_URL);
+    });
+
+    it.each([
+        {
+            errcode: undefined,
+            text: "The rageshake server encountered an unknown error and could not handle the report.",
+        },
+        {
+            errcode: "CUSTOM_ERROR_TYPE",
+            text: "The rageshake server encountered an unknown error and could not handle the report.",
+        },
+        {
+            errcode: "DISALLOWED_APP",
+            text: "Your bug report was rejected. The rageshake server does not support this application.",
+        },
+        {
+            errcode: "REJECTED_BAD_VERSION",
+            text: "Your bug report was rejected as the version you are running is too old.",
+        },
+        {
+            errcode: "REJECTED_UNEXPECTED_RECOVERY_KEY",
+            text: "Your bug report was rejected for safety reasons, as it contained a recovery key.",
+        },
+        {
+            errcode: "REJECTED_CUSTOM_REASON",
+            text: "Your bug report was rejected. The rageshake server rejected the contents of the report due to a policy.",
+        },
+    ])("handles bug report upload errors ($errcode)", async ({ errcode, text }) => {
+        const { getByLabelText, getByText } = renderComponent();
+        fetchMock.postOnce(BUG_REPORT_URL, { status: 400, body: errcode ? { errcode: errcode, error: "blah" } : "" });
+        await userEvent.type(getByLabelText("GitHub issue"), "https://example.org/some/issue");
+        await userEvent.type(getByLabelText("Notes"), "Additional text");
+        await userEvent.click(getByText("Send logs"));
+        expect(onFinished).not.toHaveBeenCalled();
+        expect(fetchMock).toHaveFetched(BUG_REPORT_URL);
+        await waitFor(() => getByText(text));
+    });
+
+    it("should show a policy link when provided", async () => {
+        const { getByLabelText, getByText } = renderComponent();
+        fetchMock.postOnce(BUG_REPORT_URL, {
+            status: 404,
+            body: { errcode: "REJECTED_CUSTOM_REASON", error: "blah", policy_url: "https://example.org/policyurl" },
+        });
+        await userEvent.type(getByLabelText("GitHub issue"), "https://example.org/some/issue");
+        await userEvent.type(getByLabelText("Notes"), "Additional text");
+        await userEvent.click(getByText("Send logs"));
+        expect(onFinished).not.toHaveBeenCalled();
+        expect(fetchMock).toHaveFetched(BUG_REPORT_URL);
+        await waitFor(() => {
+            const learnMoreLink = getByText("Learn more");
+            expect(learnMoreLink).toBeInTheDocument();
+            expect(learnMoreLink.getAttribute("href")).toEqual("https://example.org/policyurl");
+        });
+    });
+});
