Skip to content

Commit 088d4bc

Browse files
Gnathonicclaude
andcommitted
fix(mega): delete sidecars with the volume; stop sc-poll crash; fix delete toast
Three issues found deleting a MEGA-backed volume: - Sidecars (.mokuro, thumbnail .webp) were orphaned: deleteFile removes only one node. Add unifiedCloudManager.deleteManagedVolume() which deletes the archive + all managed sidecars (sidecars first, .cbz last so a sidecar failure leaves the volume retryable). VolumeItem's two delete paths use it. - Toast 'Cannot read properties of undefined (reading provider)': onBackupClicked read the reactive cloudFile after the await re-derived it to undefined. Capture the provider before the await. - megajs 'Cannot read properties of undefined (reading parent)' crash: the keepalive server-change (sc) poll has a buggy delete handler. We never use push updates (we reload explicitly), so create sessions with keepalive:false (login + both fromJSON restore sites). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent ff803ee commit 088d4bc

6 files changed

Lines changed: 132 additions & 7 deletions

File tree

src/lib/components/VolumeItem.svelte

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -345,10 +345,10 @@
345345
deleteVolumeStats(volume.volume_uuid);
346346
}
347347
348-
// Delete from cloud if checkbox checked
348+
// Delete from cloud if checkbox checked (archive + sidecars)
349349
if (deleteCloud && hasCloudBackup && cloudFile) {
350350
try {
351-
await unifiedCloudManager.deleteFile(cloudFile);
351+
await unifiedCloudManager.deleteManagedVolume(volume.series_title, volume.volume_title);
352352
showSnackbar(`Deleted from ${providerDisplayName}`);
353353
} catch (error) {
354354
console.error('Failed to delete from cloud:', error);
@@ -468,9 +468,12 @@
468468
469469
// If already backed up, delete from cloud
470470
if (isBackedUp && cloudFile) {
471+
// Capture provider before the await: cloudFile is a $derived that becomes
472+
// undefined once the delete refreshes the cache.
473+
const providerType = cloudFile.provider;
471474
try {
472-
await unifiedCloudManager.deleteFile(cloudFile);
473-
const providerName = cloudFile.provider === 'google-drive' ? 'Drive' : cloudFile.provider;
475+
await unifiedCloudManager.deleteManagedVolume(volume.series_title, volume.volume_title);
476+
const providerName = providerType === 'google-drive' ? 'Drive' : providerType;
474477
showSnackbar(`Deleted from ${providerName}`);
475478
} catch (error) {
476479
console.error('Delete failed:', error);

src/lib/util/sync/core/providers/mega-core.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,12 @@ async function getUploadStorage(session: string): Promise<Storage> {
3636

3737
uploadSessionKey = sessionKey;
3838
const pendingSession = (async () => {
39-
const storage = Storage.fromJSON(parsed) as any;
39+
// keepalive:false: no server-change poll in the worker session (we don't need it,
40+
// and its handler crashes on delete events).
41+
const storage = Storage.fromJSON({
42+
...parsed,
43+
options: { ...(parsed.options ?? {}), keepalive: false }
44+
}) as any;
4045
// fromJSON loads no tree; reload populates storage.root for folder navigation/upload.
4146
await storage.reload(true);
4247
return storage as Storage;

src/lib/util/sync/providers/mega/mega-provider.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,15 @@ describe('MegaProvider.login()', () => {
9393
expect(storageState.lastOptions.secondFactorCode).toBe('654321');
9494
});
9595

96+
it('logs in with keepalive disabled (no crashing server-change poll)', async () => {
97+
const provider = new MegaProvider();
98+
await provider.whenReady();
99+
100+
await provider.login({ email: 'a@b.c', password: 'secret' });
101+
102+
expect(storageState.lastOptions.keepalive).toBe(false);
103+
});
104+
96105
it('maps EMFAREQUIRED to a MFA_REQUIRED ProviderError', async () => {
97106
storageState.loginError = new Error('EMFAREQUIRED (-26): Multi-Factor Authentication Required');
98107
const provider = new MegaProvider();

src/lib/util/sync/providers/mega/mega-provider.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,9 +260,12 @@ export class MegaProvider implements SyncProvider {
260260

261261
// Fresh interactive login. The constructor cb fires after the tree loads
262262
// (autoload:true), so the Storage is ready once this promise resolves.
263+
// keepalive:false disables megajs's server-change (sc) long-poll. We never use
264+
// push notifications (we reload explicitly), and that poll's handler crashes on
265+
// delete events ("Cannot read properties of undefined (reading 'parent')").
263266
const storage: any = await new Promise((resolve, reject) => {
264267
const s = new Storage(
265-
{ email, password, secondFactorCode, autoload: true } as any,
268+
{ email, password, secondFactorCode, autoload: true, keepalive: false } as any,
266269
(error: Error | null) => (error ? reject(error) : resolve(s))
267270
);
268271
});
@@ -325,7 +328,12 @@ export class MegaProvider implements SyncProvider {
325328
/** Rebuild an authenticated Storage from a saved session blob (no password, no login round-trip). */
326329
private async restoreSession(blob: MegaSessionBlob): Promise<void> {
327330
const { Storage } = await import('megajs');
328-
const storage: any = Storage.fromJSON(blob as any);
331+
// Force keepalive:false so the restored session never starts the crashing sc poll,
332+
// even for blobs persisted before that default changed.
333+
const storage: any = Storage.fromJSON({
334+
...(blob as any),
335+
options: { ...((blob as any).options ?? {}), keepalive: false }
336+
});
329337
// fromJSON does no network and loads no tree; reload populates root + files.
330338
// A dead session throws ESID here.
331339
await storage.reload(true);

src/lib/util/sync/unified-cloud-manager.test.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,3 +173,63 @@ describe('UnifiedCloudManager rename operations', () => {
173173
);
174174
});
175175
});
176+
177+
describe('UnifiedCloudManager.deleteManagedVolume', () => {
178+
beforeEach(() => {
179+
vi.clearAllMocks();
180+
});
181+
182+
const baseFiles = (): CloudFileMetadata[] => [
183+
{ provider: 'mega', fileId: 'cbz-1', path: 'S/Vol 1.cbz', modifiedTime: '', size: 100 },
184+
{ provider: 'mega', fileId: 'mokuro-1', path: 'S/Vol 1.mokuro', modifiedTime: '', size: 10 },
185+
{ provider: 'mega', fileId: 'thumb-1', path: 'S/Vol 1.webp', modifiedTime: '', size: 5 },
186+
{ provider: 'mega', fileId: 'other-1', path: 'S/Vol 2.cbz', modifiedTime: '', size: 100 }
187+
];
188+
189+
it('deletes the archive and all sidecars (archive last) and clears the cache', async () => {
190+
const cache = { removeById: vi.fn() };
191+
const deleted: string[] = [];
192+
const provider = {
193+
type: 'mega',
194+
deleteFile: vi.fn(async (file: CloudFileMetadata) => {
195+
deleted.push(file.path);
196+
})
197+
};
198+
const files = baseFiles();
199+
getActiveProvider.mockReturnValue(provider);
200+
getBySeries.mockImplementation((s: string) => files.filter((f) => f.path.startsWith(`${s}/`)));
201+
getCache.mockReturnValue(cache);
202+
203+
const { unifiedCloudManager } = await import('$lib/util/sync/unified-cloud-manager');
204+
await unifiedCloudManager.deleteManagedVolume('S', 'Vol 1');
205+
206+
// Only Vol 1's three files (not Vol 2), and the .cbz archive is deleted LAST.
207+
expect(provider.deleteFile).toHaveBeenCalledTimes(3);
208+
expect(deleted).not.toContain('S/Vol 2.cbz');
209+
expect(deleted[deleted.length - 1]).toBe('S/Vol 1.cbz');
210+
expect(cache.removeById).toHaveBeenCalledTimes(3);
211+
});
212+
213+
it('reports a summary on partial failure but still clears the successes', async () => {
214+
const cache = { removeById: vi.fn() };
215+
const provider = {
216+
type: 'mega',
217+
deleteFile: vi.fn(async (file: CloudFileMetadata) => {
218+
if (file.path.endsWith('.mokuro')) throw new Error('boom');
219+
})
220+
};
221+
const files = baseFiles();
222+
getActiveProvider.mockReturnValue(provider);
223+
getBySeries.mockImplementation((s: string) => files.filter((f) => f.path.startsWith(`${s}/`)));
224+
getCache.mockReturnValue(cache);
225+
226+
const { unifiedCloudManager } = await import('$lib/util/sync/unified-cloud-manager');
227+
await expect(unifiedCloudManager.deleteManagedVolume('S', 'Vol 1')).rejects.toThrow(
228+
/Failed to delete 1 of 3/
229+
);
230+
// The .cbz and .webp still got removed from cache; only the .mokuro failed.
231+
expect(cache.removeById).toHaveBeenCalledWith('cbz-1');
232+
expect(cache.removeById).toHaveBeenCalledWith('thumb-1');
233+
expect(cache.removeById).not.toHaveBeenCalledWith('mokuro-1');
234+
});
235+
});

src/lib/util/sync/unified-cloud-manager.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,46 @@ class UnifiedCloudManager {
197197
}
198198
}
199199

200+
/**
201+
* Delete a backed-up volume and ALL its managed cloud files (archive + sidecars).
202+
* deleteFile() removes only a single node, which leaves the .mokuro and thumbnail
203+
* sidecars orphaned. Sidecars are deleted first and the .cbz archive last, so a
204+
* sidecar failure leaves the volume still marked backed-up (and retryable) rather
205+
* than half-deleted.
206+
*/
207+
async deleteManagedVolume(seriesTitle: string, volumeTitle: string): Promise<void> {
208+
const provider = this.getActiveProvider();
209+
if (!provider) {
210+
throw new Error('No cloud provider authenticated');
211+
}
212+
213+
const files = this.getManagedCloudFilesForVolume(seriesTitle, volumeTitle);
214+
if (files.length === 0) return;
215+
216+
const ordered = [...files].sort(
217+
(a, b) =>
218+
Number(normalizeCloudPath(a.path).endsWith('.cbz')) -
219+
Number(normalizeCloudPath(b.path).endsWith('.cbz'))
220+
);
221+
222+
const cache = cacheManager.getCache(provider.type);
223+
const failures: string[] = [];
224+
for (const file of ordered) {
225+
try {
226+
await provider.deleteFile(file);
227+
cache?.removeById?.(file.fileId);
228+
} catch (error) {
229+
failures.push(`${file.path}: ${error instanceof Error ? error.message : 'error'}`);
230+
}
231+
}
232+
233+
if (failures.length > 0) {
234+
throw new Error(
235+
`Failed to delete ${failures.length} of ${files.length} file(s): ${failures.join('; ')}`
236+
);
237+
}
238+
}
239+
200240
private replaceCachedFile(oldFile: CloudFileMetadata, updatedFile: CloudFileMetadata): void {
201241
const provider = this.getActiveProvider();
202242
if (!provider) return;

0 commit comments

Comments
 (0)