From d4c55c3fac4cbbd961ff08eedbabd88cafe1f3b1 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Sun, 4 Aug 2024 20:25:53 +0200 Subject: [PATCH] improve error message and types --- src/renderer/src/App.tsx | 271 +++++++++++++++-------------- src/renderer/src/dialogs/index.tsx | 7 + src/renderer/src/ffmpeg.ts | 5 +- src/renderer/src/util.ts | 50 ++++-- 4 files changed, 176 insertions(+), 157 deletions(-) diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index a05f24b8..27948a58 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -69,15 +69,16 @@ import { getOutPath, getSuffixedOutPath, handleError, getOutDir, isStoreBuild, dragPreventer, havePermissionToReadFile, resolvePathIfNeeded, getPathReadAccessError, html5ifiedPrefix, html5dummySuffix, findExistingHtml5FriendlyFile, - deleteFiles, isOutOfSpaceError, isExecaFailure, readFileSize, readFileSizes, checkFileSizes, setDocumentTitle, getOutFileExtension, getSuffixedFileName, mustDisallowVob, readVideoTs, readDirRecursively, getImportProjectType, - calcShouldShowWaveform, calcShouldShowKeyframes, mediaSourceQualities, getFrameDuration, + deleteFiles, isOutOfSpaceError, readFileSize, readFileSizes, checkFileSizes, setDocumentTitle, getOutFileExtension, getSuffixedFileName, mustDisallowVob, readVideoTs, readDirRecursively, getImportProjectType, + calcShouldShowWaveform, calcShouldShowKeyframes, mediaSourceQualities, getFrameDuration, isExecaError, getStdioString, + isMuxNotSupported, } from './util'; import { toast, errorToast } from './swal'; import { formatDuration, parseDuration } from './util/duration'; import { adjustRate } from './util/rate-calculator'; import { askExtractFramesAsImages } from './dialogs/extractFrames'; import { askForHtml5ifySpeed } from './dialogs/html5ify'; -import { askForOutDir, askForImportChapters, promptTimeOffset, askForFileOpenAction, confirmExtractAllStreamsDialog, showCleanupFilesDialog, showDiskFull, showExportFailedDialog, showConcatFailedDialog, openYouTubeChaptersDialog, showRefuseToOverwrite, openDirToast, openExportFinishedToast, openConcatFinishedToast, showOpenDialog } from './dialogs'; +import { askForOutDir, askForImportChapters, promptTimeOffset, askForFileOpenAction, confirmExtractAllStreamsDialog, showCleanupFilesDialog, showDiskFull, showExportFailedDialog, showConcatFailedDialog, openYouTubeChaptersDialog, showRefuseToOverwrite, openDirToast, openExportFinishedToast, openConcatFinishedToast, showOpenDialog, showMuxNotSupported } from './dialogs'; import { openSendReportDialog } from './reporting'; import { fallbackLng } from './i18n'; import { createSegment, getCleanCutSegments, findSegmentsAtCursor, sortSegments, convertSegmentsToChapters, hasAnySegmentOverlap, isDurationValid, playOnlyCurrentSegment, getSegmentTags } from './segments'; @@ -485,7 +486,7 @@ function App() { // New LLC format can be stored along with input file or in working dir (customOutDir) const getEdlFilePath = useCallback((fp?: string, cod?: string) => getSuffixedOutPath({ customOutDir: cod, filePath: fp, nameSuffix: projectSuffix }), []); // Old versions of LosslessCut used CSV files and stored them always in customOutDir: - const getEdlFilePathOld = useCallback((fp, cod) => getSuffixedOutPath({ customOutDir: cod, filePath: fp, nameSuffix: oldProjectSuffix }), []); + const getEdlFilePathOld = useCallback((fp: string | undefined, cod?: string | undefined) => getSuffixedOutPath({ customOutDir: cod, filePath: fp, nameSuffix: oldProjectSuffix }), []); const getProjectFileSavePath = useCallback((storeProjectInWorkingDirIn: boolean) => getEdlFilePath(filePath, storeProjectInWorkingDirIn ? customOutDir : undefined), [getEdlFilePath, filePath, customOutDir]); const projectFileSavePath = useMemo(() => getProjectFileSavePath(storeProjectInWorkingDir), [getProjectFileSavePath, storeProjectInWorkingDir]); @@ -1071,17 +1072,17 @@ function App() { openSendReportDialog(err, state); }, [commonSettings, copyStreamIdsByFile, cutSegments, effectiveExportMode, externalFilesMeta, fileFormat, filePath, mainFileFormatData, mainStreams, outSegTemplate, rotation, shortestFlag]); - const openSendConcatReportDialogWithState = useCallback(async (err, reportState) => { + const openSendConcatReportDialogWithState = useCallback(async (err: unknown, reportState?: object) => { const state = { ...commonSettings, ...reportState }; openSendReportDialog(err, state); }, [commonSettings]); - const handleExportFailed = useCallback(async (err) => { + const handleExportFailed = useCallback(async (err: unknown) => { const sendErrorReport = await showExportFailedDialog({ fileFormat, safeOutputFileName }); if (sendErrorReport) openSendReportDialogWithState(err); }, [fileFormat, safeOutputFileName, openSendReportDialogWithState]); - const handleConcatFailed = useCallback(async (err, reportState) => { + const handleConcatFailed = useCallback(async (err: unknown, reportState: object) => { const sendErrorReport = await showConcatFailedDialog({ fileFormat }); if (sendErrorReport) openSendConcatReportDialogWithState(err, reportState); }, [fileFormat, openSendConcatReportDialogWithState]); @@ -1129,24 +1130,26 @@ function App() { } catch (err) { if (err instanceof DirectoryAccessDeclinedError) return; - if (err instanceof Error) { - if ('killed' in err && err.killed === true) { + if (isExecaError(err)) { + if (err.killed) { // assume execa killed (aborted by user) return; } - if ('stdout' in err) console.error('stdout:', err.stdout); - if ('stderr' in err) console.error('stderr:', err.stderr); + console.log('stdout:', getStdioString(err.stdout)); + console.error('stderr:', getStdioString(err.stderr)); - if (isExecaFailure(err)) { - if (isOutOfSpaceError(err)) { - showDiskFull(); - return; - } - const reportState = { includeAllStreams, streams, outFormat, outFileName, segmentsToChapters }; - handleConcatFailed(err, reportState); + if (isOutOfSpaceError(err)) { + showDiskFull(); return; } + if (isMuxNotSupported(err)) { + showMuxNotSupported(); + return; + } + const reportState = { includeAllStreams, streams, outFormat, outFileName, segmentsToChapters }; + handleConcatFailed(err, reportState); + return; } handleError(err); @@ -1350,25 +1353,25 @@ function App() { resetMergedOutFileName(); } catch (err) { - if (err instanceof Error) { - if ('killed' in err && err.killed === true) { + if (isExecaError(err)) { + if (err.killed) { // assume execa killed (aborted by user) return; } - // @ts-expect-error todo - if ('stdout' in err && err.stdout != null) console.error('stdout:', err.stdout.toString('utf8')); - // @ts-expect-error todo - if ('stderr' in err && err.stderr != null) console.error('stderr:', err.stderr.toString('utf8')); + console.log('stdout:', getStdioString(err.stdout)); + console.error('stderr:', getStdioString(err.stderr)); - if (isExecaFailure(err)) { - if (isOutOfSpaceError(err)) { - showDiskFull(); - return; - } - handleExportFailed(err); + if (isOutOfSpaceError(err)) { + showDiskFull(); + return; + } + if (isMuxNotSupported(err)) { + showMuxNotSupported(); return; } + handleExportFailed(err); + return; } handleError(err); @@ -1480,8 +1483,8 @@ function App() { } }, [filePath, loadCutSegments, setWorking]); - const loadMedia = useCallback(async ({ filePath: fp, projectPath }: { filePath: string, projectPath?: string }) => { - async function tryOpenProjectPath(path, type) { + const loadMedia = useCallback(async ({ filePath: fp, projectPath }: { filePath: string, projectPath?: string | undefined }) => { + async function tryOpenProjectPath(path: string, type: EdlFileType) { if (!(await exists(path))) return false; await loadEdlFile({ path, type }); return true; @@ -1650,7 +1653,7 @@ function App() { const userOpenSingleFile = useCallback(async ({ path: pathIn, isLlcProject }: { path: string, isLlcProject?: boolean }) => { let path = pathIn; - let projectPath; + let projectPath: string | undefined; // Open .llc AND media referenced within if (isLlcProject) { @@ -1900,133 +1903,131 @@ function App() { }, []); const userOpenFiles = useCallback(async (filePathsIn?: string[]) => { - let filePaths = filePathsIn; - if (!filePaths || filePaths.length === 0) return; + try { + let filePaths = filePathsIn; + if (!filePaths || filePaths.length === 0) return; - console.log('userOpenFiles'); - console.log(filePaths.join('\n')); + console.log('userOpenFiles'); + console.log(filePaths.join('\n')); - lastOpenedPathRef.current = filePaths[0]!; + lastOpenedPathRef.current = filePaths[0]!; - // first check if it is a single directory, and if so, read it recursively - if (filePaths.length === 1) { - const firstFilePath = filePaths[0]; - const firstFileStat = await lstat(firstFilePath); - if (firstFileStat.isDirectory()) { - console.log('Reading directory...'); - filePaths = await readDirRecursively(firstFilePath); + // first check if it is a single directory, and if so, read it recursively + if (filePaths.length === 1) { + const firstFilePath = filePaths[0]; + const firstFileStat = await lstat(firstFilePath); + if (firstFileStat.isDirectory()) { + console.log('Reading directory...'); + filePaths = await readDirRecursively(firstFilePath); + } } - } - // Only allow opening regular files - // eslint-disable-next-line no-restricted-syntax - for (const path of filePaths) { - // eslint-disable-next-line no-await-in-loop - const fileStat = await lstat(path); + // Only allow opening regular files + // eslint-disable-next-line no-restricted-syntax + for (const path of filePaths) { + // eslint-disable-next-line no-await-in-loop + const fileStat = await lstat(path); - if (!fileStat.isFile()) { - errorToast(i18n.t('Cannot open anything else than regular files')); - console.warn('Not a file:', path); - return; + if (!fileStat.isFile()) { + errorToast(i18n.t('Cannot open anything else than regular files')); + console.warn('Not a file:', path); + return; + } } - } - if (filePaths.length > 1) { - if (alwaysConcatMultipleFiles) { - batchLoadPaths(filePaths); - setConcatDialogVisible(true); - } else { - batchLoadPaths(filePaths, true); + if (filePaths.length > 1) { + if (alwaysConcatMultipleFiles) { + batchLoadPaths(filePaths); + setConcatDialogVisible(true); + } else { + batchLoadPaths(filePaths, true); + } + return; } - return; - } - // filePaths.length is now 1 - const [firstFilePath] = filePaths; - invariant(firstFilePath != null); + // filePaths.length is now 1 + const [firstFilePath] = filePaths; + invariant(firstFilePath != null); - // https://en.wikibooks.org/wiki/Inside_DVD-Video/Directory_Structure - if (/^video_ts$/i.test(basename(firstFilePath))) { - if (mustDisallowVob()) return; - filePaths = await readVideoTs(firstFilePath); - } + // https://en.wikibooks.org/wiki/Inside_DVD-Video/Directory_Structure + if (/^video_ts$/i.test(basename(firstFilePath))) { + if (mustDisallowVob()) return; + filePaths = await readVideoTs(firstFilePath); + } - if (workingRef.current) return; - try { - setWorking({ text: i18n.t('Loading file') }); + if (workingRef.current) return; + try { + setWorking({ text: i18n.t('Loading file') }); - // Import segments for for already opened file - const matchingImportProjectType = getImportProjectType(firstFilePath); - if (matchingImportProjectType) { - if (!checkFileOpened()) return; - await loadEdlFile({ path: firstFilePath, type: matchingImportProjectType, append: true }); - return; - } + // Import segments for for already opened file + const matchingImportProjectType = getImportProjectType(firstFilePath); + if (matchingImportProjectType) { + if (!checkFileOpened()) return; + await loadEdlFile({ path: firstFilePath, type: matchingImportProjectType, append: true }); + return; + } - const filePathLowerCase = firstFilePath.toLowerCase(); - const isLlcProject = filePathLowerCase.endsWith('.llc'); + const filePathLowerCase = firstFilePath.toLowerCase(); + const isLlcProject = filePathLowerCase.endsWith('.llc'); - // Need to ask the user what to do if more than one option - const inputOptions: { open: string, project?: string, tracks?: string, subtitles?: string, addToBatch?: string, mergeWithCurrentFile?: string } = { - open: isFileOpened ? i18n.t('Open the file instead of the current one') : i18n.t('Open the file'), - }; + // Need to ask the user what to do if more than one option + const inputOptions: { open: string, project?: string, tracks?: string, subtitles?: string, addToBatch?: string, mergeWithCurrentFile?: string } = { + open: isFileOpened ? i18n.t('Open the file instead of the current one') : i18n.t('Open the file'), + }; - if (isFileOpened) { - if (isLlcProject) inputOptions.project = i18n.t('Load segments from the new file, but keep the current media'); - if (filePathLowerCase.endsWith('.srt')) inputOptions.subtitles = i18n.t('Convert subtitiles into segments'); - inputOptions.tracks = i18n.t('Include all tracks from the new file'); - } + if (isFileOpened) { + if (isLlcProject) inputOptions.project = i18n.t('Load segments from the new file, but keep the current media'); + if (filePathLowerCase.endsWith('.srt')) inputOptions.subtitles = i18n.t('Convert subtitiles into segments'); + inputOptions.tracks = i18n.t('Include all tracks from the new file'); + } - if (batchFiles.length > 0) inputOptions.addToBatch = i18n.t('Add the file to the batch list'); - else if (isFileOpened) inputOptions.mergeWithCurrentFile = i18n.t('Merge/concatenate with current file'); + if (batchFiles.length > 0) inputOptions.addToBatch = i18n.t('Add the file to the batch list'); + else if (isFileOpened) inputOptions.mergeWithCurrentFile = i18n.t('Merge/concatenate with current file'); - if (Object.keys(inputOptions).length > 1) { - const openFileResponse = enableAskForFileOpenAction ? await askForFileOpenAction(inputOptions) : 'open'; + if (Object.keys(inputOptions).length > 1) { + const openFileResponse = enableAskForFileOpenAction ? await askForFileOpenAction(inputOptions) : 'open'; - if (openFileResponse === 'open') { - await userOpenSingleFile({ path: firstFilePath, isLlcProject }); - return; - } - if (openFileResponse === 'project') { - await loadEdlFile({ path: firstFilePath, type: 'llc' }); - return; - } - if (openFileResponse === 'subtitles') { - await loadEdlFile({ path: firstFilePath, type: 'srt' }); - return; - } - if (openFileResponse === 'tracks') { - await addStreamSourceFile(firstFilePath); - setStreamsSelectorShown(true); - return; - } - if (openFileResponse === 'addToBatch') { - batchLoadPaths([firstFilePath], true); - return; - } - if (openFileResponse === 'mergeWithCurrentFile') { - const batchPaths = new Set(); - if (filePath) batchPaths.add(filePath); - filePaths.forEach((path) => batchPaths.add(path)); - batchLoadPaths([...batchPaths]); - if (batchPaths.size > 1) setConcatDialogVisible(true); + if (openFileResponse === 'open') { + await userOpenSingleFile({ path: firstFilePath, isLlcProject }); + return; + } + if (openFileResponse === 'project') { + await loadEdlFile({ path: firstFilePath, type: 'llc' }); + return; + } + if (openFileResponse === 'subtitles') { + await loadEdlFile({ path: firstFilePath, type: 'srt' }); + return; + } + if (openFileResponse === 'tracks') { + await addStreamSourceFile(firstFilePath); + setStreamsSelectorShown(true); + return; + } + if (openFileResponse === 'addToBatch') { + batchLoadPaths([firstFilePath], true); + return; + } + if (openFileResponse === 'mergeWithCurrentFile') { + const batchPaths = new Set(); + if (filePath) batchPaths.add(filePath); + filePaths.forEach((path) => batchPaths.add(path)); + batchLoadPaths([...batchPaths]); + if (batchPaths.size > 1) setConcatDialogVisible(true); + return; + } + + // Dialog canceled: return; } - // Dialog canceled: - return; + await userOpenSingleFile({ path: firstFilePath, isLlcProject }); + } finally { + setWorking(undefined); } - - await userOpenSingleFile({ path: firstFilePath, isLlcProject }); } catch (err) { console.error('userOpenFiles', err); - if (err instanceof Error && 'code' in err && err.code === 'LLC_FFPROBE_UNSUPPORTED_FILE') { - errorToast(i18n.t('Unsupported file')); - } else { - handleError(i18n.t('Failed to open file'), err); - } - } finally { - setWorking(undefined); + handleError(i18n.t('Failed to open file'), err); } }, [alwaysConcatMultipleFiles, batchLoadPaths, setWorking, isFileOpened, batchFiles.length, userOpenSingleFile, checkFileOpened, loadEdlFile, enableAskForFileOpenAction, addStreamSourceFile, filePath]); diff --git a/src/renderer/src/dialogs/index.tsx b/src/renderer/src/dialogs/index.tsx index eb6aa2a4..1cb545bc 100644 --- a/src/renderer/src/dialogs/index.tsx +++ b/src/renderer/src/dialogs/index.tsx @@ -145,6 +145,13 @@ export async function showDiskFull() { }); } +export async function showMuxNotSupported() { + await Swal.fire({ + icon: 'error', + text: i18n.t('At least one codec is not supported by the selected output file format. Try another output format or try to disable one or more tracks.'), + }); +} + export async function showRefuseToOverwrite() { await Swal.fire({ icon: 'warning', diff --git a/src/renderer/src/ffmpeg.ts b/src/renderer/src/ffmpeg.ts index 524957df..658906e3 100644 --- a/src/renderer/src/ffmpeg.ts +++ b/src/renderer/src/ffmpeg.ts @@ -6,7 +6,7 @@ import minBy from 'lodash/minBy'; import invariant from 'tiny-invariant'; import { pcmAudioCodecs, getMapStreamsArgs, isMov, LiteFFprobeStream } from './util/streams'; -import { getSuffixedOutPath, isExecaFailure } from './util'; +import { getSuffixedOutPath, isExecaError } from './util'; import { isDurationValid } from './segments'; import { FFprobeChapter, FFprobeFormat, FFprobeProbeResult, FFprobeStream } from '../../../ffprobe'; import { parseSrt } from './edlFormats'; @@ -350,8 +350,7 @@ export async function readFileMeta(filePath: string) { invariant(format != null); return { format, streams, chapters }; } catch (err) { - // Windows will throw error with code ENOENT if format detection fails. - if (isExecaFailure(err)) { + if (isExecaError(err)) { throw Object.assign(new Error(`Unsupported file: ${err.message}`), { code: 'LLC_FFPROBE_UNSUPPORTED_FILE' }); } throw err; diff --git a/src/renderer/src/util.ts b/src/renderer/src/util.ts index dd5b8692..43b6f82c 100644 --- a/src/renderer/src/util.ts +++ b/src/renderer/src/util.ts @@ -10,7 +10,7 @@ import type * as FsExtra from 'fs-extra'; import type { PlatformPath } from 'node:path'; import isDev from './isDev'; -import Swal, { toast } from './swal'; +import Swal, { errorToast, toast } from './swal'; import { ffmpegExtractWindow } from './util/constants'; const { dirname, parse: parsePath, join, extname, isAbsolute, resolve, basename }: PlatformPath = window.require('path'); @@ -162,19 +162,24 @@ export async function transferTimestamps({ inPath, outPath, cutFrom = 0, cutTo = export function handleError(arg1: unknown, arg2?: unknown) { console.error('handleError', arg1, arg2); - let msg; - let errorMsg; - if (typeof arg1 === 'string') msg = arg1; - else if (typeof arg2 === 'string') msg = arg2; + let err: Error | undefined; + let str: string | undefined; - if (arg1 instanceof Error) errorMsg = arg1.message; - if (arg2 instanceof Error) errorMsg = arg2.message; + if (typeof arg1 === 'string') str = arg1; + else if (typeof arg2 === 'string') str = arg2; - toast.fire({ - icon: 'error', - title: msg || i18n.t('An error has occurred.'), - text: errorMsg ? errorMsg.slice(0, 300) : undefined, - }); + if (arg1 instanceof Error) err = arg1; + else if (arg2 instanceof Error) err = arg2; + + if (err != null && 'code' in err && err.code === 'LLC_FFPROBE_UNSUPPORTED_FILE') { + errorToast(i18n.t('Unsupported file')); + } else { + toast.fire({ + icon: 'error', + title: str || i18n.t('An error has occurred.'), + text: err?.message ? err?.message.slice(0, 300) : undefined, + }); + } } export function filenamify(name: string) { @@ -306,18 +311,25 @@ export const deleteDispositionValue = 'llc_disposition_remove'; export const mirrorTransform = 'matrix(-1, 0, 0, 1, 0, 0)'; -export function isExecaError(err: unknown): err is Pick { +export type InvariantExecaError = ExecaError | ExecaError | ExecaError; + +// note: I don't think we can use instanceof ExecaError because the error has been sent over the main-renderer bridge +export function isExecaError(err: unknown): err is InvariantExecaError { return err instanceof Error && 'stdout' in err && 'stderr' in err; } -// I *think* Windows will throw error with code ENOENT if ffprobe/ffmpeg fails (execa), but other OS'es will return this error code if a file is not found, so it would be wrong to attribute it to exec failure. -// see https://github.com/mifi/lossless-cut/issues/451 -export const isExecaFailure = (err): err is ExecaError => err.exitCode === 1 || (isWindows && err.code === 'ENOENT'); +export const getStdioString = (stdio: string | Buffer | undefined) => (stdio instanceof Buffer ? stdio.toString('utf8') : stdio); // A bit hacky but it works, unless someone has a file called "No space left on device" ( ͡° ͜ʖ ͡°) -export const isOutOfSpaceError = (err): err is ExecaError => ( - err && isExecaFailure(err) - && typeof err.stderr === 'string' && err.stderr.includes('No space left on device') +export const isOutOfSpaceError = (err: InvariantExecaError) => ( + err.exitCode === 1 + && !!getStdioString(err.stderr)?.includes('No space left on device') +); + +export const isMuxNotSupported = (err: InvariantExecaError) => ( + err.exitCode === 1 + && err.stderr != null + && /Could not write header .*incorrect codec parameters .*Invalid argument/.test(getStdioString(err.stderr) ?? '') ); export async function checkAppPath() {