From d3f9ced5dcd153baaff3b202cdf951b5e0b1fe1b Mon Sep 17 00:00:00 2001 From: Nico <47644445+nicotsx@users.noreply.github.com> Date: Thu, 15 Jan 2026 22:09:00 +0100 Subject: [PATCH] refactor: use spawn & exec instead of bun $ (#356) * refactor: use spawn & exec accordingly * chore: pr feedbacks --- .../modules/backends/rclone/rclone-backend.ts | 4 +- .../modules/backends/utils/backend-utils.ts | 7 +- .../backups/__tests__/backups.service.test.ts | 12 +-- .../migrations/00001-retag-snapshots.ts | 4 +- app/server/utils/rclone.ts | 6 +- app/server/utils/restic.ts | 73 ++++++------- app/server/utils/shoutrrr.ts | 7 +- app/server/utils/spawn.ts | 102 ++++++++++-------- vite.config.ts | 2 +- 9 files changed, 108 insertions(+), 109 deletions(-) diff --git a/app/server/modules/backends/rclone/rclone-backend.ts b/app/server/modules/backends/rclone/rclone-backend.ts index 7c1cdfd2..9048edc2 100644 --- a/app/server/modules/backends/rclone/rclone-backend.ts +++ b/app/server/modules/backends/rclone/rclone-backend.ts @@ -1,6 +1,5 @@ import * as fs from "node:fs/promises"; import * as os from "node:os"; -import { $ } from "bun"; import { OPERATION_TIMEOUT } from "../../../core/constants"; import { toMessage } from "../../../utils/errors"; import { logger } from "../../../utils/logger"; @@ -9,6 +8,7 @@ import { withTimeout } from "../../../utils/timeout"; import type { VolumeBackend } from "../backend"; import { executeUnmount } from "../utils/backend-utils"; import { BACKEND_STATUS, type BackendConfig } from "~/schemas/volumes"; +import { exec } from "~/server/utils/spawn"; const mount = async (config: BackendConfig, path: string) => { logger.debug(`Mounting rclone volume ${path}...`); @@ -50,7 +50,7 @@ const mount = async (config: BackendConfig, path: string) => { logger.debug(`Mounting rclone volume ${path}...`); logger.info(`Executing rclone: rclone ${args.join(" ")}`); - const result = await $`rclone ${args}`.nothrow(); + const result = await exec({ command: "rclone", args }); if (result.exitCode !== 0) { const errorMsg = result.stderr.toString() || result.stdout.toString() || "Unknown error"; diff --git a/app/server/modules/backends/utils/backend-utils.ts b/app/server/modules/backends/utils/backend-utils.ts index 044f4a7e..8d9685fe 100644 --- a/app/server/modules/backends/utils/backend-utils.ts +++ b/app/server/modules/backends/utils/backend-utils.ts @@ -1,8 +1,8 @@ import * as fs from "node:fs/promises"; import * as npath from "node:path"; -import { $ } from "bun"; import { toMessage } from "../../../utils/errors"; import { logger } from "../../../utils/logger"; +import { exec } from "~/server/utils/spawn"; export const executeMount = async (args: string[]): Promise => { const shouldBeVerbose = process.env.LOG_LEVEL === "debug" || process.env.NODE_ENV !== "production"; @@ -10,7 +10,7 @@ export const executeMount = async (args: string[]): Promise => { const effectiveArgs = shouldBeVerbose && !hasVerboseFlag ? ["-v", ...args] : args; logger.debug(`Executing mount ${effectiveArgs.join(" ")}`); - let result = await $`mount ${effectiveArgs}`.nothrow(); + const result = await exec({ command: "mount", args: effectiveArgs, timeout: 10000 }); const stdout = result.stdout.toString().trim(); const stderr = result.stderr.toString().trim(); @@ -31,7 +31,8 @@ export const executeUnmount = async (path: string): Promise => { let stderr: string | undefined; logger.debug(`Executing umount -l ${path}`); - const result = await $`umount -l ${path}`.nothrow(); + const result = await exec({ command: "umount", args: ["-l", path], timeout: 10000 }); + stderr = result.stderr.toString(); if (stderr?.trim()) { diff --git a/app/server/modules/backups/__tests__/backups.service.test.ts b/app/server/modules/backups/__tests__/backups.service.test.ts index 2c70f82e..d99c5819 100644 --- a/app/server/modules/backups/__tests__/backups.service.test.ts +++ b/app/server/modules/backups/__tests__/backups.service.test.ts @@ -7,7 +7,7 @@ import { generateBackupOutput } from "~/test/helpers/restic"; import { faker } from "@faker-js/faker"; import * as spawnModule from "~/server/utils/spawn"; -const resticBackupMock = mock(() => Promise.resolve({ exitCode: 0, stdout: "", stderr: "" })); +const resticBackupMock = mock(() => Promise.resolve({ exitCode: 0, summary: "", error: "" })); beforeEach(() => { resticBackupMock.mockClear(); @@ -31,7 +31,7 @@ describe("execute backup", () => { expect(schedule.nextBackupAt).toBeNull(); resticBackupMock.mockImplementationOnce(() => - Promise.resolve({ exitCode: 0, stdout: generateBackupOutput(), stderr: "" }), + Promise.resolve({ exitCode: 0, summary: generateBackupOutput(), error: "" }), ); // act @@ -76,7 +76,7 @@ describe("execute backup", () => { }); resticBackupMock.mockImplementationOnce(() => - Promise.resolve({ exitCode: 0, stdout: generateBackupOutput(), stderr: "" }), + Promise.resolve({ exitCode: 0, summary: generateBackupOutput(), error: "" }), ); // act @@ -97,7 +97,7 @@ describe("execute backup", () => { resticBackupMock.mockImplementation(async () => { await new Promise((resolve) => setTimeout(resolve, 100)); - return Promise.resolve({ exitCode: 0, stdout: generateBackupOutput(), stderr: "" }); + return Promise.resolve({ exitCode: 0, summary: generateBackupOutput(), error: "" }); }); // act @@ -119,7 +119,7 @@ describe("execute backup", () => { }); resticBackupMock.mockImplementationOnce(() => - Promise.resolve({ exitCode: 3, stdout: generateBackupOutput(), stderr: "Some error occurred" }), + Promise.resolve({ exitCode: 3, summary: generateBackupOutput(), error: "Some error occurred" }), ); // act @@ -140,7 +140,7 @@ describe("execute backup", () => { }); resticBackupMock.mockImplementationOnce(() => - Promise.resolve({ exitCode: 1, stdout: generateBackupOutput(), stderr: "Some error occurred" }), + Promise.resolve({ exitCode: 1, summary: generateBackupOutput(), error: "Some error occurred" }), ); // act diff --git a/app/server/modules/lifecycle/migrations/00001-retag-snapshots.ts b/app/server/modules/lifecycle/migrations/00001-retag-snapshots.ts index 234922f8..c156112c 100644 --- a/app/server/modules/lifecycle/migrations/00001-retag-snapshots.ts +++ b/app/server/modules/lifecycle/migrations/00001-retag-snapshots.ts @@ -3,7 +3,7 @@ import { db } from "../../../db/db"; import { backupScheduleMirrorsTable, repositoriesTable, type Repository } from "../../../db/schema"; import { logger } from "../../../utils/logger"; import { toMessage } from "~/server/utils/errors"; -import { safeSpawn } from "~/server/utils/spawn"; +import { exec } from "~/server/utils/spawn"; import { addCommonArgs, buildEnv, buildRepoUrl, cleanupTemporaryKeys } from "~/server/utils/restic"; const migrateTag = async ( @@ -20,7 +20,7 @@ const migrateTag = async ( addCommonArgs(args, env); logger.info(`Migrating snapshots for schedule '${scheduleName}' from tag '${oldTag}' to '${newTag}'`); - const res = await safeSpawn({ command: "restic", args, env }); + const res = await exec({ command: "restic", args, env }); await cleanupTemporaryKeys(env); if (res.exitCode !== 0) { diff --git a/app/server/utils/rclone.ts b/app/server/utils/rclone.ts index 4e6470d0..30378327 100644 --- a/app/server/utils/rclone.ts +++ b/app/server/utils/rclone.ts @@ -1,13 +1,13 @@ -import { $ } from "bun"; import { logger } from "./logger"; import { toMessage } from "./errors"; +import { exec } from "./spawn"; /** * List all configured rclone remotes * @returns Array of remote names */ export async function listRcloneRemotes(): Promise { - const result = await $`rclone listremotes`.nothrow(); + const result = await exec({ command: "rclone", args: ["listremotes"] }); if (result.exitCode !== 0) { logger.error(`Failed to list rclone remotes: ${result.stderr.toString()}`); @@ -34,7 +34,7 @@ export async function getRcloneRemoteInfo( remote: string, ): Promise<{ type: string; config: Record } | null> { try { - const result = await $`rclone config show ${remote}`.quiet(); + const result = await exec({ command: "rclone", args: ["config", "show", remote] }); if (result.exitCode !== 0) { logger.error(`Failed to get info for remote ${remote}: ${result.stderr.toString()}`); diff --git a/app/server/utils/restic.ts b/app/server/utils/restic.ts index 733c2e1d..dd5f7b64 100644 --- a/app/server/utils/restic.ts +++ b/app/server/utils/restic.ts @@ -9,7 +9,7 @@ import { config as appConfig } from "../core/config"; import { logger } from "./logger"; import { cryptoUtils } from "./crypto"; import type { RetentionPolicy } from "../modules/backups/backups.dto"; -import { safeSpawn } from "./spawn"; +import { safeSpawn, exec } from "./spawn"; import type { CompressionMode, RepositoryConfig, OverwriteMode, BandwidthLimit } from "~/schemas/restic"; import { ResticError } from "./errors"; @@ -231,7 +231,7 @@ const init = async (config: RepositoryConfig) => { const args = ["init", "--repo", repoUrl]; addCommonArgs(args, env, config); - const res = await safeSpawn({ command: "restic", args, env }); + const res = await exec({ command: "restic", args, env }); await cleanupTemporaryKeys(env); if (res.exitCode !== 0) { @@ -341,8 +341,6 @@ const backup = async ( } }, 1000); - let stdout = ""; - logger.debug(`Executing: restic ${args.join(" ")}`); const res = await safeSpawn({ command: "restic", @@ -350,37 +348,34 @@ const backup = async ( env, signal: options?.signal, onStdout: (data) => { - stdout = data; logData(data); - if (options?.onProgress) { streamProgress(data); } }, - finally: async () => { - if (includeFile) await fs.unlink(includeFile).catch(() => {}); - if (excludeFile) await fs.unlink(excludeFile).catch(() => {}); - await cleanupTemporaryKeys(env); - }, }); + if (includeFile) await fs.unlink(includeFile).catch(() => {}); + if (excludeFile) await fs.unlink(excludeFile).catch(() => {}); + await cleanupTemporaryKeys(env); + if (options?.signal?.aborted) { logger.warn("Restic backup was aborted by signal."); return { result: null, exitCode: res.exitCode }; } if (res.exitCode === 3) { - logger.error(`Restic backup encountered read errors: ${res.stderr}`); + logger.error(`Restic backup encountered read errors: ${res.error}`); } if (res.exitCode !== 0 && res.exitCode !== 3) { - logger.error(`Restic backup failed: ${res.stderr}`); + logger.error(`Restic backup failed: ${res.error}`); logger.error(`Command executed: restic ${args.join(" ")}`); - throw new ResticError(res.exitCode, res.stderr); + throw new ResticError(res.exitCode, res.error); } - const lastLine = (stdout || res.stdout).trim(); + const lastLine = res.summary.trim(); let summaryLine = ""; try { const resSummary = JSON.parse(lastLine ?? "{}"); @@ -390,6 +385,7 @@ const backup = async ( summaryLine = "{}"; } + logger.debug(`Restic backup output last line: ${summaryLine}`); const result = backupOutputSchema(summaryLine); if (result instanceof type.errors) { @@ -461,27 +457,22 @@ const restore = async ( await cleanupTemporaryKeys(env); if (res.exitCode !== 0) { - logger.error(`Restic restore failed: ${res.stderr}`); - throw new ResticError(res.exitCode, res.stderr); + logger.error(`Restic restore failed: ${res.error}`); + throw new ResticError(res.exitCode, res.error); } - const outputLines = res.stdout.trim().split("\n"); - const lastLine = outputLines[outputLines.length - 1]; - - if (!lastLine) { - logger.info(`Restic restore completed for snapshot ${snapshotId} to target ${target}`); - return { - message_type: "summary" as const, - total_files: 0, - files_restored: 0, - files_skipped: 0, - bytes_skipped: 0, - }; + const lastLine = res.summary.trim(); + let summaryLine = ""; + try { + const resSummary = JSON.parse(lastLine ?? "{}"); + summaryLine = resSummary; + } catch (_) { + logger.warn("Failed to parse restic backup output JSON summary.", lastLine); + summaryLine = "{}"; } - logger.debug(`Restic restore output last line: ${lastLine}`); - const resSummary = JSON.parse(lastLine); - const result = restoreOutputSchema(resSummary); + logger.debug(`Restic restore output last line: ${summaryLine}`); + const result = restoreOutputSchema(summaryLine); if (result instanceof type.errors) { logger.warn(`Restic restore output validation failed: ${result.summary}`); @@ -518,7 +509,7 @@ const snapshots = async (config: RepositoryConfig, options: { tags?: string[] } addCommonArgs(args, env, config); - const res = await safeSpawn({ command: "restic", args, env }); + const res = await exec({ command: "restic", args, env }); await cleanupTemporaryKeys(env); if (res.exitCode !== 0) { @@ -567,7 +558,7 @@ const forget = async (config: RepositoryConfig, options: RetentionPolicy, extra: args.push("--prune"); addCommonArgs(args, env, config); - const res = await safeSpawn({ command: "restic", args, env }); + const res = await exec({ command: "restic", args, env }); await cleanupTemporaryKeys(env); if (res.exitCode !== 0) { @@ -589,7 +580,7 @@ const deleteSnapshots = async (config: RepositoryConfig, snapshotIds: string[]) const args: string[] = ["--repo", repoUrl, "forget", ...snapshotIds, "--prune"]; addCommonArgs(args, env, config); - const res = await safeSpawn({ command: "restic", args, env }); + const res = await exec({ command: "restic", args, env }); await cleanupTemporaryKeys(env); if (res.exitCode !== 0) { @@ -638,7 +629,7 @@ const tagSnapshots = async ( addCommonArgs(args, env, config); - const res = await safeSpawn({ command: "restic", args, env }); + const res = await exec({ command: "restic", args, env }); await cleanupTemporaryKeys(env); if (res.exitCode !== 0) { @@ -688,7 +679,7 @@ const ls = async (config: RepositoryConfig, snapshotId: string, path?: string) = addCommonArgs(args, env, config); - const res = await safeSpawn({ command: "restic", args, env }); + const res = await exec({ command: "restic", args, env }); await cleanupTemporaryKeys(env); if (res.exitCode !== 0) { @@ -739,7 +730,7 @@ const unlock = async (config: RepositoryConfig) => { const args = ["unlock", "--repo", repoUrl, "--remove-all"]; addCommonArgs(args, env, config); - const res = await safeSpawn({ command: "restic", args, env }); + const res = await exec({ command: "restic", args, env }); await cleanupTemporaryKeys(env); if (res.exitCode !== 0) { @@ -763,7 +754,7 @@ const check = async (config: RepositoryConfig, options?: { readData?: boolean }) addCommonArgs(args, env, config); - const res = await safeSpawn({ command: "restic", args, env }); + const res = await exec({ command: "restic", args, env }); await cleanupTemporaryKeys(env); const { stdout, stderr } = res; @@ -796,7 +787,7 @@ const repairIndex = async (config: RepositoryConfig) => { const args = ["repair", "index", "--repo", repoUrl]; addCommonArgs(args, env, config); - const res = await safeSpawn({ command: "restic", args, env }); + const res = await exec({ command: "restic", args, env }); await cleanupTemporaryKeys(env); const { stdout, stderr } = res; @@ -863,7 +854,7 @@ const copy = async ( logger.info(`Copying snapshots from ${sourceRepoUrl} to ${destRepoUrl}...`); logger.debug(`Executing: restic ${args.join(" ")}`); - const res = await safeSpawn({ command: "restic", args, env }); + const res = await exec({ command: "restic", args, env }); await cleanupTemporaryKeys(sourceEnv); await cleanupTemporaryKeys(destEnv); diff --git a/app/server/utils/shoutrrr.ts b/app/server/utils/shoutrrr.ts index 18475c83..f6cd0e17 100644 --- a/app/server/utils/shoutrrr.ts +++ b/app/server/utils/shoutrrr.ts @@ -1,4 +1,4 @@ -import { safeSpawn } from "./spawn"; +import { exec } from "./spawn"; import { logger } from "./logger"; import { toMessage } from "./errors"; @@ -16,10 +16,7 @@ export async function sendNotification(params: SendNotificationParams) { logger.debug(`Sending notification via Shoutrrr: ${title}`); - const result = await safeSpawn({ - command: "shoutrrr", - args, - }); + const result = await exec({ command: "shoutrrr", args }); if (result.exitCode === 0) { logger.debug(`Notification sent successfully: ${title}`); diff --git a/app/server/utils/spawn.ts b/app/server/utils/spawn.ts index c2b9246b..a68f321d 100644 --- a/app/server/utils/spawn.ts +++ b/app/server/utils/spawn.ts @@ -1,78 +1,88 @@ -import { spawn } from "node:child_process"; +import { spawn, execFile, type ExecException, type ExecFileOptions } from "node:child_process"; +import { createInterface } from "node:readline"; +import { promisify } from "node:util"; + +type ExecProps = { + command: string; + args?: string[]; + env?: NodeJS.ProcessEnv; +} & ExecFileOptions; + +export const exec = async ({ command, args = [], env = {}, ...rest }: ExecProps) => { + const options = { + env: { ...process.env, ...env }, + }; + + try { + const { stdout, stderr } = await promisify(execFile)(command, args, { ...options, ...rest, encoding: "utf8" }); + + return { exitCode: 0, stdout, stderr }; + } catch (error) { + const execError = error as ExecException; + + return { + exitCode: typeof execError.code === "number" ? execError.code : 1, + stdout: execError.stdout || "", + stderr: execError.stderr || "", + }; + } +}; export interface SafeSpawnParams { command: string; args: string[]; env?: NodeJS.ProcessEnv; signal?: AbortSignal; - onStdout?: (data: string) => void; + onStdout?: (line: string) => void; onStderr?: (error: string) => void; - onError?: (error: Error) => Promise | void; - onClose?: (code: number | null) => Promise | void; - finally?: () => Promise | void; } type SpawnResult = { exitCode: number; - stdout: string; - stderr: string; + summary: string; + error: string; }; export const safeSpawn = (params: SafeSpawnParams) => { - const { command, args, env = {}, signal, ...callbacks } = params; + const { command, args, env = {}, signal, onStdout, onStderr } = params; + + let lastStdout = ""; + let lastStderr = ""; return new Promise((resolve) => { - let stdoutData = ""; - let stderrData = ""; - const child = spawn(command, args, { env: { ...process.env, ...env }, signal: signal, }); - child.stdout.on("data", (data) => { - if (callbacks.onStdout) { - callbacks.onStdout(data.toString()); - } else { - stdoutData += data.toString(); + child.stdout.setEncoding("utf8"); + child.stderr.setEncoding("utf8"); + + const rl = createInterface({ input: child.stdout }); + const rlErr = createInterface({ input: child.stderr }); + + rl.on("line", (line) => { + if (onStdout) onStdout(line); + const trimmed = line.trim(); + if (trimmed.length > 0) { + lastStdout = line; } }); - child.stderr.on("data", (data) => { - if (callbacks.onStderr) { - callbacks.onStderr(data.toString()); + rlErr.on("line", (line) => { + if (onStderr) onStderr(line); + const trimmed = line.trim(); + if (trimmed.length > 0) { + lastStderr = line; } - stderrData += data.toString(); }); - child.on("error", async (error) => { - if (callbacks.onError) { - await callbacks.onError(error); - } - if (callbacks.finally) { - await callbacks.finally(); - } - - resolve({ - exitCode: -1, - stdout: stdoutData, - stderr: stderrData, - }); + child.on("error", (err) => { + resolve({ exitCode: -1, summary: lastStdout, error: err.message || lastStderr }); }); - child.on("close", async (code) => { - if (callbacks.onClose) { - await callbacks.onClose(code); - } - if (callbacks.finally) { - await callbacks.finally(); - } - - resolve({ - exitCode: code === null ? -1 : code, - stdout: stdoutData, - stderr: stderrData, - }); + child.on("close", (code) => { + resolve({ exitCode: code ?? -1, summary: lastStdout, error: lastStderr }); }); }); }; diff --git a/vite.config.ts b/vite.config.ts index a21be80a..910ccc06 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -14,7 +14,7 @@ export default defineConfig({ }, }, server: { - host: true, + host: '0.0.0.0', port: 4096, }, });