Skip to content

Commit

Permalink
[bugfix-738] show better error messages with fs errors
Browse files Browse the repository at this point in the history
* fix is only applied to fs.writeFile on creating desktop
  • Loading branch information
silentrald committed Jan 30, 2025
1 parent 2e9f62d commit 00d6af1
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 18 deletions.
6 changes: 6 additions & 0 deletions assets/jsons/translations/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@
"generic": {
"env": {
"parse": "Could not properly parse the env var string."
},
"fs": {
"write-file": "Could not write file \"{filepath}\".",
"flatpak": {
"write-file": "Could not write file \"{filepath}\". Check if the parent folder is writable in your flatpak permissions."
}
}
},
"title-bar": {
Expand Down
9 changes: 9 additions & 0 deletions src/__tests__/unit/fs.helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,15 @@ import { mkdir, pathExistsSync, rm, writeFile } from "fs-extra";
import { getSize } from "main/helpers/fs.helpers";
import path from "path";

jest.mock("electron", () => ({ app: {
getPath: () => "",
getName: () => "",
}}));
jest.mock("electron-log", () => ({
info: jest.fn(),
error: jest.fn(),
}));

const TEST_FOLDER = path.resolve(__dirname, "..", "assets", "fs");

describe("Test fs.helpers getSize", () => {
Expand Down
20 changes: 19 additions & 1 deletion src/main/helpers/fs.helpers.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { CopyOptions, MoveOptions, copy, createReadStream, ensureDir, move, pathExists, pathExistsSync, realpath, stat, symlink } from "fs-extra";
import fs, { CopyOptions, MoveOptions, copy, createReadStream, ensureDir, move, pathExists, pathExistsSync, realpath, stat, symlink } from "fs-extra";
import { access, mkdir, rm, readdir, unlink, lstat, readlink } from "fs/promises";
import path from "path";
import { Observable, concatMap, from } from "rxjs";
Expand All @@ -9,6 +9,9 @@ import { execSync } from "child_process";
import { tryit } from "../../shared/helpers/error.helpers";
import { CustomError } from "shared/models/exceptions/custom-error.class";
import { ErrorObject } from "serialize-error";
import { IS_FLATPAK } from "main/constants";

// NOTE: For future fs errors, add generic.fs.<name> to properly show them as notification to the user

export async function pathExist(path: string): Promise<boolean> {
try {
Expand Down Expand Up @@ -318,6 +321,21 @@ export async function getSize(targetPath: string, maxDepth = 5): Promise<number>
return computeSize(targetPath, 0);
}

// fs errors in flatpak may be a result of sandbox permissions, a separate error message is needed.
const createResourceKey = (prefix: string, name: string) => IS_FLATPAK
? `${prefix}.flatpak.${name}`
: `${prefix}.${name}`;

export const writeFile = async (
filepath: number | fs.PathLike,
data: any,
options?: string | fs.WriteFileOptions
) => fs.writeFile(filepath, data, options).catch((error: Error) => {
throw CustomError.fromError(error, createResourceKey("generic.fs", "write-file"), {
params: { filepath }
});
});

export interface Progression<T = unknown, D = unknown> {
total: number;
current: number;
Expand Down
19 changes: 10 additions & 9 deletions src/main/services/linux.service.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import fs from "fs-extra";
import fsExtra from "fs-extra";
import * as fs from "../helpers/fs.helpers";
import log from "electron-log";
import path from "path";
import { BS_APP_ID, BS_EXECUTABLE, IS_FLATPAK, PROTON_BINARY_PREFIX, WINE_BINARY_PREFIX } from "main/constants";
Expand Down Expand Up @@ -71,7 +72,7 @@ export class LinuxService {
this.staticConfig.get("proton-folder"),
PROTON_BINARY_PREFIX
);
if (!fs.pathExistsSync(protonPath)) {
if (!fsExtra.pathExistsSync(protonPath)) {
throw CustomError.fromError(
new Error("Could not locate proton binary"),
BSLaunchError.PROTON_NOT_FOUND
Expand All @@ -91,9 +92,9 @@ export class LinuxService {
// using bsmanager, it won't exist, and proton will fail
// to launch the game.
const compatDataPath = this.getCompatDataPath();
if (!fs.existsSync(compatDataPath)) {
if (!fsExtra.existsSync(compatDataPath)) {
log.info(`Proton compat data path not found at '${compatDataPath}', creating directory`);
await fs.ensureDir(compatDataPath);
await fsExtra.ensureDir(compatDataPath);
}

// Setup Proton environment variables
Expand Down Expand Up @@ -126,7 +127,7 @@ export class LinuxService {

const protonPath = path.join(protonFolder, PROTON_BINARY_PREFIX);
const winePath = path.join(protonFolder, WINE_BINARY_PREFIX);
return fs.pathExistsSync(protonPath) && fs.pathExistsSync(winePath);
return fsExtra.pathExistsSync(protonPath) && fsExtra.pathExistsSync(winePath);
}

public getWinePath(): string {
Expand All @@ -138,7 +139,7 @@ export class LinuxService {
this.staticConfig.get("proton-folder"),
WINE_BINARY_PREFIX
);
if (!fs.pathExistsSync(winePath)) {
if (!fsExtra.pathExistsSync(winePath)) {
throw new Error(`"${winePath}" binary file not found`);
}

Expand All @@ -147,7 +148,7 @@ export class LinuxService {

public getWinePrefixPath(): string {
const compatDataPath = this.getCompatDataPath();
return fs.existsSync(compatDataPath)
return fsExtra.existsSync(compatDataPath)
? path.join(compatDataPath, "pfx") : "";
}

Expand Down Expand Up @@ -224,9 +225,9 @@ export class LinuxService {
await fs.writeFile(shortcutPath, desktopEntry);
log.info("Created shorcut at ", `"${shortcutPath}/${name}"`);
return true;
} catch (error) {
} catch (error: any) {
log.error("Could not create shortcut", error);
return false;
throw CustomError.fromError(error);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ import BeatConflictImg from "../../../../assets/images/apngs/beat-conflict.png";
import BeatWaitingImg from "../../../../assets/images/apngs/beat-waiting.png";
import BeatImpatientImg from "../../../../assets/images/apngs/beat-impatient.png";
import { BsmButton } from "../shared/bsm-button.component";
import { useTranslation } from "renderer/hooks/use-translation.hook";
import { useTranslationV2 } from "renderer/hooks/use-translation.hook";
import { ForwardedRef, forwardRef } from "react";

export const NotificationItem = forwardRef(({ resolver, notification }: { resolver?: (value: NotificationResult | string) => void; notification: Notification }, fwdRef: ForwardedRef<HTMLLIElement>) => {
const t = useTranslation();
const { text: t } = useTranslationV2();

const renderImage = (() => {
if (notification.type === NotificationType.SUCCESS) {
Expand Down Expand Up @@ -44,7 +44,7 @@ export const NotificationItem = forwardRef(({ resolver, notification }: { resolv
return "bg-gray-800 shadow-gray-800 dark:bg-white dark:shadow-white";
})();

const handleDragEnd = (e: MouseEvent, info: PanInfo) => {
const handleDragEnd = (_e: MouseEvent, info: PanInfo) => {
const offset = info.offset.x;
const velocity = info.velocity.x;

Expand Down
6 changes: 3 additions & 3 deletions src/renderer/pages/version-viewer.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -169,11 +169,11 @@ export function VersionViewer() {
title: "notifications.create-launch-shortcut.success.title",
desc: `notifications.create-launch-shortcut.success.${data.steamShortcut ? "msg-steam" : "msg"}`
});
}).catch(() => {
}).catch(error => {
notification.notifyError({
title: "notifications.types.error",
desc: "notifications.create-launch-shortcut.error.msg"
});
desc: "notifications.create-launch-shortcut.error.msg",
}, error);
});
}

Expand Down
7 changes: 5 additions & 2 deletions src/renderer/services/notification.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,16 @@ export class NotificationService {

promise.then(() => {
this.notifications$.next(this.notifications$.value.filter(n => n.id !== resovableNotification.id));
});
});

return promise;
}

public notifyError(notification: Notification): Promise<NotificationResult | string> {
public notifyError(notification: Notification, error?: Error): Promise<NotificationResult | string> {
notification.type = NotificationType.ERROR;
if ((error as any)?.code) {
notification.desc = (error as any).code;
}
return this.notify(notification);
}

Expand Down

0 comments on commit 00d6af1

Please sign in to comment.