gsadmin/Firelink
1
0
Fork 0
mirror of https://github.com/nimbold/Firelink.git synced 2026-07-26 12:08:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(downloads): harden add modal and retry state

Browse Source 
Route explicit no-limit speed overrides without inheriting the global cap.

Preserve dotted media titles when switching formats and drain retry gid completions through remember_gid.
This commit is contained in:
NimBold
2026-07-08 21:57:12 +03:30
parent f84726a403
commit 161b0028f9
9 changed files with 205 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src-tauri/src/lib.rs
+13 -12
View File
@@ -2837,10 +2837,11 @@ pub(crate) async fn start_media_download_internal(
.arg(output_template.to_string_lossy().to_string())
.env("PATH", &trusted_path);
if let Some(limit) = speed_limit.as_ref() {
if !limit.is_empty() {
cmd = cmd.arg("--limit-rate").arg(limit);
}
if let Some(limit) = speed_limit
.as_deref()
.and_then(normalize_speed_limit_for_aria2)
{
cmd = cmd.arg("--limit-rate").arg(limit);
}
if let Some(p) = proxy.as_deref().map(str::trim).filter(|s| !s.is_empty()) {
@@ -3280,6 +3281,13 @@ async fn resume_download(
queue_manager.release_permit(&id_clone).await;
return;
}
let _ = app_handle_clone.emit(
"download-state",
crate::ipc::DownloadStateEvent::new(
&id_clone,
crate::ipc::DownloadStatus::Downloading,
),
);
let result = match rpc_call(
aria2_port,
&aria2_secret,
@@ -3332,13 +3340,6 @@ async fn resume_download(
return;
}
log::info!("aria2 resume [{}]: unpaused gid {}", id_clone, gid_clone);
let _ = app_handle_clone.emit(
"download-state",
crate::ipc::DownloadStateEvent::new(
&id_clone,
crate::ipc::DownloadStatus::Downloading,
),
);
});
return Ok(true);
}
@@ -3970,7 +3971,7 @@ async fn set_concurrent_limit(
Ok(())
}
fn normalize_speed_limit_for_aria2(limit: &str) -> Option<String> {
pub(crate) fn normalize_speed_limit_for_aria2(limit: &str) -> Option<String> {
let trimmed = limit.trim();
if trimmed.is_empty() {
return None;
src-tauri/src/queue.rs
+9 -19
View File
@@ -596,20 +596,6 @@ impl<R: tauri::Runtime> QueueManager<R> {
removed.last().cloned()
}
/// Overwrite a stale aria2 gid with the fresh gid minted by a retry
/// `addUri`. Failing to call this after re-add leaks the semaphore permit.
pub fn rotate_aria2_gid(&self, id: &str, stale_gid: &str, new_gid: &str) {
let mut gids = self.aria2_gids.write().unwrap();
gids.remove(stale_gid);
gids.insert(new_gid.to_string(), id.to_string());
log::info!(
"aria2 gid transition [{}]: rotated {} -> {}",
id,
stale_gid,
new_gid
);
}
async fn wait_permit_released(self: &Arc<Self>, id: &str) {
loop {
if !self.active_permits.lock().await.contains_key(id) {
@@ -695,7 +681,6 @@ impl<R: tauri::Runtime> QueueManager<R> {
}
let this = Arc::clone(self);
let stale_gid = gid.to_string();
let id_for_task = id.clone();
let error_for_emit = error.clone();
tauri::async_runtime::spawn(async move {
@@ -752,11 +737,12 @@ impl<R: tauri::Runtime> QueueManager<R> {
);
return;
}
this.rotate_aria2_gid(&id_for_task, &stale_gid, &new_gid);
let retained_gid = new_gid.clone();
this.remember_gid(id_for_task.clone(), new_gid).await;
log::warn!(
"aria2 retry cancellation [{}]: retained late gid {} mapping for remove retry",
id_for_task,
new_gid
retained_gid
);
return;
}
@@ -764,8 +750,8 @@ impl<R: tauri::Runtime> QueueManager<R> {
.lock()
.await
.insert(id_for_task.clone(), strike + 1);
this.rotate_aria2_gid(&id_for_task, &stale_gid, &new_gid);
this.emit_state(&id_for_task, DownloadStatus::Downloading);
this.remember_gid(id_for_task.clone(), new_gid).await;
}
Err(retry_error) => {
this.apply_completion(&id_for_task, PendingOutcome::Error(retry_error))
@@ -1204,7 +1190,11 @@ impl SidecarSpawner for ProductionSpawner {
options.insert("max-tries".to_string(), serde_json::json!(mt.to_string()));
options.insert("retry-wait".to_string(), serde_json::json!("2"));
options.insert("continue".to_string(), serde_json::json!("true"));
if let Some(speed) = &payload.speed_limit {
if let Some(speed) = payload
.speed_limit
.as_deref()
.and_then(crate::normalize_speed_limit_for_aria2)
{
options.insert("max-download-limit".to_string(), serde_json::json!(speed));
}
if let Some(user) = &payload.username {
src-tauri/src/retry.rs
+6 -7
View File
@@ -18,15 +18,14 @@
//! ### aria2 GID-rotation contract (CRITICAL)
//!
//! When aria2 retries via a fresh `aria2.addUri`, it mints a **brand-new GID**.
//! The caller MUST overwrite the stale GID → download-id mapping in
//! `QueueManager::aria2_gids` with the new GID on every successful re-add.
//! Failing to do so detaches subsequent `onDownloadComplete` /
//! `onDownloadError` WebSocket events from the original id, which leaks the
//! semaphore permit permanently. Concretely, after every retry-driven
//! `addUri` that returns `new_gid`:
//! The caller MUST store the new GID through `QueueManager::remember_gid` on
//! every successful re-add. Failing to do so detaches subsequent
//! `onDownloadComplete` / `onDownloadError` WebSocket events from the original
//! id, or strands a terminal event that arrived before the fresh GID was stored.
//! Concretely, after every retry-driven `addUri` that returns `new_gid`:
//!
//! ```ignore
//! queue_manager.rotate_aria2_gid(&id, &stale_gid, &new_gid);
//! queue_manager.remember_gid(id.clone(), new_gid).await;
//! ```
use std::time::Duration;
src/components/AddDownloadsModal.tsx
+19 -31
View File
@@ -22,6 +22,7 @@ import { isTransferLocked } from '../utils/downloadActions';
import { useToast } from '../contexts/ToastContext';
import {
canSubmitMetadataRows,
mediaFileNameForSelectedFormat,
mediaFormatSelectorForRow,
metadataSummaryMessage,
reconcileDownloadRows,
@@ -387,7 +388,7 @@ export const AddDownloadsModal = () => {
}
})();
}
}, [parsedItems, pendingAddFilename, password, useAuth, username, headers, cookies]);
}, [parsedItems, pendingAddFilename, pendingAddMediaUrls]);
useEffect(() => {
if (parsedItems.length === 0) {
@@ -481,12 +482,9 @@ export const AddDownloadsModal = () => {
for (let i = 0; i < parsedItems.length; i++) {
const item = parsedItems[i];
let finalFile = canonicalizeDownloadFileName(item.file);
if (item.isMedia && item.formats && item.selectedFormat !== undefined) {
const selectedFormat = item.formats[item.selectedFormat];
const baseName = finalFile.substring(0, finalFile.lastIndexOf('.')) || finalFile;
finalFile = `${baseName}.${selectedFormat.ext}`;
}
let finalFile = item.isMedia
? mediaFileNameForSelectedFormat(item.file, item)
: canonicalizeDownloadFileName(item.file);
const itemLocation = useSharedDestination
? finalLocation
: destinationOverrides[i] || await categoryLocationForFile(finalFile);
@@ -577,12 +575,9 @@ export const AddDownloadsModal = () => {
if (res.resolution === 'skip') {
itemsToAdd[idx] = null;
} else if (res.resolution === 'rename') {
let finalFile = canonicalizeDownloadFileName(item.file);
if (item.isMedia && item.formats && item.selectedFormat !== undefined) {
const selectedFormat = item.formats[item.selectedFormat];
const baseName = finalFile.substring(0, finalFile.lastIndexOf('.')) || finalFile;
finalFile = `${baseName}.${selectedFormat.ext}`;
}
let finalFile = item.isMedia
? mediaFileNameForSelectedFormat(item.file, item)
: canonicalizeDownloadFileName(item.file);
const itemLocation = useSharedDestination
? finalLocation
: destinationOverrides[idx] || await categoryLocationForFile(finalFile);
@@ -633,12 +628,9 @@ export const AddDownloadsModal = () => {
itemsToAdd[idx] = null;
continue;
}
let finalFile = canonicalizeDownloadFileName(item.file);
if (item.isMedia && item.formats && item.selectedFormat !== undefined) {
const selectedFormat = item.formats[item.selectedFormat];
const baseName = finalFile.substring(0, finalFile.lastIndexOf('.')) || finalFile;
finalFile = `${baseName}.${selectedFormat.ext}`;
}
let finalFile = item.isMedia
? mediaFileNameForSelectedFormat(item.file, item)
: canonicalizeDownloadFileName(item.file);
const itemLocation = useSharedDestination
? finalLocation
: destinationOverrides[idx] || await categoryLocationForFile(finalFile);
@@ -682,17 +674,11 @@ export const AddDownloadsModal = () => {
if (!item) continue;
try {
const id = crypto.randomUUID();
let finalFile = canonicalizeDownloadFileName(item.file);
let finalFile = item.isMedia
? mediaFileNameForSelectedFormat(item.file, item)
: canonicalizeDownloadFileName(item.file);
let formatSelector = mediaFormatSelectorForRow(item);
if (item.isMedia && item.formats && item.selectedFormat !== undefined) {
const selectedFormat = item.formats[item.selectedFormat];
if (!finalFile.endsWith(`.${selectedFormat.ext}`)) {
const baseName = finalFile.substring(0, finalFile.lastIndexOf('.')) || finalFile;
finalFile = `${baseName}.${selectedFormat.ext}`;
}
}
const category = categoryForFileName(finalFile);
const added = await addDownload({
id,
@@ -701,7 +687,7 @@ export const AddDownloadsModal = () => {
category,
dateAdded: new Date().toISOString(),
connections: Number(connections),
speedLimit: speedLimitEnabled ? `${speedLimit}K` : undefined,
speedLimit: speedLimitEnabled ? `${speedLimit}K` : '0',
username: useAuth ? username.trim() : undefined,
password: useAuth ? password.trim() : undefined,
headers: headers.trim() || undefined,
@@ -763,7 +749,6 @@ export const AddDownloadsModal = () => {
const format = selectedItem?.formats?.[index];
if (!selectedItem || !format) return;
const baseName = selectedItem.file.substring(0, selectedItem.file.lastIndexOf('.')) || selectedItem.file;
setParsedItems(items => items.map((item, itemIndex) =>
itemIndex === selectedItemIndex
? {
@@ -771,7 +756,10 @@ export const AddDownloadsModal = () => {
selectedFormat: index,
size: format.bytes ? format.detail : undefined,
sizeBytes: format.bytes || undefined,
file: canonicalizeDownloadFileName(`${baseName}.${format.ext}`)
file: mediaFileNameForSelectedFormat(item.file, {
formats: item.formats,
selectedFormat: index
})
}
: item
));
src/index.css
+1
View File
@@ -1801,6 +1801,7 @@ body.is-resizing .column-resize-handle:hover::after {
border-radius: 6px;
color: hsl(var(--text-primary));
font-size: var(--download-row-font-size);
overflow: hidden;
user-select: none;
-webkit-user-select: none;
}
src/store/useDownloadStore.test.ts
+84
View File
@@ -54,6 +54,29 @@ vi.mock('./useSettingsStore', () => ({
describe('useDownloadStore', () => {
beforeEach(() => {
vi.clearAllMocks();
vi.mocked(useSettingsStore.getState).mockReturnValue({
proxyMode: 'none',
siteLogins: [],
globalSpeedLimit: '',
speedLimitPresetValues: [1, 5, 10],
logsEnabled: false,
perServerConnections: 16,
customUserAgent: '',
maxAutomaticRetries: 3,
mediaCookieSource: 'none',
baseDownloadFolder: '~/Downloads',
categorySubfoldersEnabled: true,
categorySubfolders: {
Musics: 'Musics',
Movies: 'Movies',
Compressed: 'Compressed',
Documents: 'Documents',
Pictures: 'Pictures',
Applications: 'Applications',
Other: 'Other',
},
categoryDirectoryOverrides: {},
} as unknown as ReturnType<typeof useSettingsStore.getState>);
useDownloadStore.setState({
downloads: [],
backendRegisteredIds: new Set(),
@@ -253,6 +276,67 @@ describe('useDownloadStore', () => {
);
});
it('does not replace an explicit no-limit item speed with the global speed limit', async () => {
const defaultSettings = useSettingsStore.getState();
vi.mocked(useSettingsStore.getState).mockReturnValue({
...defaultSettings,
globalSpeedLimit: '2M'
});
vi.mocked(ipc.invokeCommand).mockImplementation(async (cmd: string) => {
if (cmd === 'get_pending_order') return ['uncapped'];
return undefined;
});
await useDownloadStore.getState().addDownload({
id: 'uncapped',
url: 'https://example.com/uncapped.bin',
fileName: 'uncapped.bin',
category: 'Other',
dateAdded: '',
speedLimit: '0'
}, { type: 'start-now' });
expect(ipc.invokeCommand).toHaveBeenCalledWith(
'enqueue_download',
expect.objectContaining({
item: expect.objectContaining({
id: 'uncapped',
speed_limit: '0'
})
})
);
});
it('uses the global speed limit only when an item has no explicit speed override', async () => {
const defaultSettings = useSettingsStore.getState();
vi.mocked(useSettingsStore.getState).mockReturnValue({
...defaultSettings,
globalSpeedLimit: '2M'
});
vi.mocked(ipc.invokeCommand).mockImplementation(async (cmd: string) => {
if (cmd === 'get_pending_order') return ['inherits-global'];
return undefined;
});
await useDownloadStore.getState().addDownload({
id: 'inherits-global',
url: 'https://example.com/inherits-global.bin',
fileName: 'inherits-global.bin',
category: 'Other',
dateAdded: ''
}, { type: 'start-now' });
expect(ipc.invokeCommand).toHaveBeenCalledWith(
'enqueue_download',
expect.objectContaining({
item: expect.objectContaining({
id: 'inherits-global',
speed_limit: '2M'
})
})
);
});
it('reports a rejected immediate start instead of claiming success', async () => {
vi.mocked(ipc.invokeCommand).mockImplementation(async (command: string) => {
if (command === 'enqueue_download') {
src/store/useDownloadStore.ts
+10 -2
View File
@@ -20,6 +20,14 @@ const backendDispatchPromises = new Map<string, Promise<boolean>>();
const errorMessage = (error: unknown): string =>
error instanceof Error ? error.message : String(error);
const speedLimitForDispatch = (itemSpeedLimit: string | undefined, globalSpeedLimit: string): string | null => {
const explicitLimit = itemSpeedLimit?.trim();
if (explicitLimit) {
return normalizeSpeedLimitForBackend(explicitLimit) || (explicitLimit === '0' ? '0' : null);
}
return normalizeSpeedLimitForBackend(globalSpeedLimit);
};
export async function dispatchItem(id: string): Promise<boolean> {
if (backendDispatchPromises.has(id)) return backendDispatchPromises.get(id)!;
@@ -50,7 +58,7 @@ export async function dispatchItem(id: string): Promise<boolean> {
destination,
filename: item.fileName,
connections: item.connections || settings.perServerConnections || null,
speed_limit: item.speedLimit || normalizeSpeedLimitForBackend(settings.globalSpeedLimit),
speed_limit: speedLimitForDispatch(item.speedLimit, settings.globalSpeedLimit),
username: item.username || (login ? login.username : null),
password: item.password || keychainPassword,
headers: item.headers || null,
@@ -860,7 +868,7 @@ export const useDownloadStore = create<DownloadState>((set, get) => ({
destination: destPath,
filename: item.fileName,
connections: item.connections || settings.perServerConnections || null,
speed_limit: item.speedLimit || normalizeSpeedLimitForBackend(settings.globalSpeedLimit),
speed_limit: speedLimitForDispatch(item.speedLimit, settings.globalSpeedLimit),
username: item.username || (login ? login.username : null),
password: item.password || keychainPassword,
headers: item.headers || null,
src/utils/addDownloadMetadata.test.ts
+31
View File
@@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest';
import {
canSubmitMetadataRows,
mediaFormatSelectorForRow,
mediaFileNameForSelectedFormat,
metadataSummaryMessage,
reconcileDownloadRows,
refreshFailedMetadataRows,
@@ -154,6 +155,36 @@ describe('add download metadata workflow', () => {
expect(mediaFormatSelectorForRow(failedMedia)).toBeUndefined();
});
it('replaces only known media container suffixes when selecting formats', () => {
const mediaRow = row({
isMedia: true,
formats: [
{
name: '1080p MP4',
selector: 'mp4',
ext: 'mp4',
formatLabel: '1080p',
detail: '10 MB',
type: 'Video',
bytes: 10
},
{
name: '1080p MKV',
selector: 'mkv',
ext: 'mkv',
formatLabel: '1080p',
detail: '11 MB',
type: 'Video',
bytes: 11
}
],
selectedFormat: 1
});
expect(mediaFileNameForSelectedFormat('Version 1.5', mediaRow)).toBe('Version 1.5.mkv');
expect(mediaFileNameForSelectedFormat('Version 1.5.mp4', mediaRow)).toBe('Version 1.5.mkv');
});
it('reports fallback and invalid states accurately', () => {
expect(metadataSummaryMessage([
row(),
src/utils/addDownloadMetadata.ts
+32
View File
@@ -148,6 +148,38 @@ export const mediaFormatSelectorForRow = (
return row.formats?.[row.selectedFormat]?.selector;
};
export const mediaFileNameForSelectedFormat = (
fileName: string,
row: Pick<AddDownloadDraftRow, 'formats' | 'selectedFormat'>
): string => {
const selectedFormat = row.selectedFormat === undefined
? undefined
: row.formats?.[row.selectedFormat];
if (!selectedFormat) return canonicalizeDownloadFileName(fileName);
const cleanFileName = canonicalizeDownloadFileName(fileName);
const selectedExt = selectedFormat.ext.replace(/^\.+/, '');
if (!selectedExt) return cleanFileName;
const lowerFileName = cleanFileName.toLowerCase();
if (lowerFileName.endsWith(`.${selectedExt.toLowerCase()}`)) {
return cleanFileName;
}
const lastDot = cleanFileName.lastIndexOf('.');
const currentExt = lastDot > 0 ? cleanFileName.slice(lastDot + 1).toLowerCase() : '';
const knownFormatExts = new Set(
(row.formats || [])
.map(format => format.ext.replace(/^\.+/, '').toLowerCase())
.filter(Boolean)
);
const baseName = currentExt && knownFormatExts.has(currentExt)
? cleanFileName.slice(0, lastDot)
: cleanFileName;
return canonicalizeDownloadFileName(`${baseName}.${selectedExt}`);
};
export const metadataSummaryMessage = (rows: AddDownloadDraftRow[]): string => {
if (rows.length === 0) return 'Paste one or more links.';