Skip to content

fix: improve error handling and add abort signal support for bulk removal #3812

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rajanarahul93 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix: improve error handling and add abort signal support for bulk removal #3812

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
62579f2
fix: improve error handling and add abort signal support for bulk rem…
rajanarahul93 Apr 26, 2026
6467275
changes
rajanarahul93 Apr 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 67 additions & 27 deletions js&css/web-accessible/www.youtube.com/playlist-complete-playlist.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ function saveLocalWatchHistory() {
} else {
localStorage.setItem(key, JSON.stringify(data));
}
} catch (e) { }
} catch (e) { console.warn('[ImprovedTube] watch history save failed', e); }
}

/**
Expand All @@ -321,7 +321,7 @@ function resetWatchForVideo(videoId, renderer) {
// Inform extension watched store (best-effort)
try {
ImprovedTube.messages.send({ action: 'watched', type: 'remove', id: videoId });
} catch (e) { }
} catch (e) { console.debug('[ImprovedTube] watch-reset message failed', e); }
}

/*------------------------------------------------------------------------------
Expand Down Expand Up @@ -470,17 +470,22 @@ async function clickMenuRemove(renderer) {
/**
* Load all playlist items by scrolling and clicking continuation buttons
* @param {Function} statusCb - Callback function for status updates
* @param {AbortSignal} [signal] - Optional signal to abort loading early
*/
async function loadAllPlaylistItems(statusCb) {
async function loadAllPlaylistItems(statusCb, signal) {
const list = document.querySelector('ytd-playlist-video-list-renderer #contents') ||
document.querySelector('ytd-playlist-video-list-renderer');
if (!list) return;
if (!list) return false;

let lastCount = 0;
let stable = 0;
const maxStable = 3;
const deadline = Date.now() + 90_000;

for (let i = 0; i < 300; i++) {
if (signal?.aborted) return false;
if (Date.now() > deadline) return true;

Comment thread
rajanarahul93 marked this conversation as resolved.
// Scroll to end to trigger loading
try { list.scrollTop = list.scrollHeight; } catch (e) { }
window.scrollTo(0, document.documentElement.scrollHeight);
Expand All @@ -497,6 +502,7 @@ async function loadAllPlaylistItems(statusCb) {
if (count <= lastCount) { stable++; } else { stable = 0; lastCount = count; }
if (stable >= maxStable && !list.querySelector('ytd-continuation-item-renderer')) break;
}
return false;
}

/**
Expand All @@ -522,48 +528,62 @@ function collectCandidates(threshold) {
/**
* Execute bulk removal of videos based on watch threshold
* @param {number} threshold - Watch percentage threshold (0-100)
* @param {AbortSignal} [signal] - Optional signal to abort early
* @returns {Promise<number>} - Number of videos removed
*/
async function runRemoval(threshold) {
async function runRemoval(threshold, signal) {
const items = collectCandidates(threshold);
if (items.length === 0) return 0;

const playlistId = new URLSearchParams(location.search).get('list');
const confirmedNodes = new Set();

// Try edit_playlist with setVideoIds first (closer to native)
let ok = true;
// Try edit_playlist with setVideoIds in chunks to avoid oversized requests
const BATCH_SIZE = 50;
let ok = false;
const setIds = items.map(i => i.setId).filter(Boolean);
const bySetId = new Map(items.filter(i => i.setId).map(i => [i.setId, i]));
if (setIds.length) {
ok = true;
for (const sid of setIds) {
const okOne = await removeVideosPersistentlyEdit(playlistId, [sid]);
ok = ok && okOne;
for (let i = 0; i < setIds.length; i += BATCH_SIZE) {
if (signal?.aborted) break;
const chunk = setIds.slice(i, i + BATCH_SIZE);
const r = await removeVideosPersistentlyEdit(playlistId, chunk);
if (!r) { ok = false; break; }
Comment thread
rajanarahul93 marked this conversation as resolved.
for (const sid of chunk) {
Comment on lines 547 to +553
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the abort signal is triggered during the edit_playlist chunk loop, the loop breaks but ok remains true. That prevents the fallback removal methods from running and makes the function look like it completed successfully even though it was cancelled mid-run. Consider explicitly treating signal.aborted as a non-successful completion (e.g., set ok = false or return early) so cancellation semantics are consistent.

Copilot uses AI. Check for mistakes.
const item = bySetId.get(sid);
if (item) confirmedNodes.add(item.node);
}
}
} else {
ok = false;
}

// Fallback to modify with removedVideoId (batch)
if (!ok) ok = await removeVideosPersistentlyModify(playlistId, items.map(i => i.id).filter(Boolean));
if (!ok && !signal?.aborted) {
ok = await removeVideosPersistentlyModify(playlistId, items.map(i => i.id).filter(Boolean));
if (ok) items.forEach(({ node }) => confirmedNodes.add(node));
}

if (!ok) {
if (!ok && !signal?.aborted) {
// Fallback to UI-driven removal (more reliable, slower)
for (const { node, id, setId } of items) {
if (signal?.aborted) break;
const did = await clickMenuRemove(node);
if (!did) removeFromPlaylist(id, setId);
confirmedNodes.add(node);
Comment on lines 570 to +572
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the UI-fallback path, confirmedNodes is updated unconditionally even when both clickMenuRemove(node) and removeFromPlaylist(id, setId) fail. Since the new behavior is to remove only confirmed items from the DOM, this can incorrectly remove items locally that weren’t actually removed from the playlist. Track the return value of removeFromPlaylist and only add the node to confirmedNodes when one of the removal methods succeeds.

Suggested change
const did = await clickMenuRemove(node);
if (!did) removeFromPlaylist(id, setId);
confirmedNodes.add(node);
let removed = await clickMenuRemove(node);
if (!removed) removed = await removeFromPlaylist(id, setId);
if (removed) confirmedNodes.add(node);

Copilot uses AI. Check for mistakes.
}
}

// Optimistically remove DOM nodes for immediate feedback
for (const { node } of items) { try { node.remove(); } catch (e) { } }
// Remove only confirmed items from the DOM
for (const node of confirmedNodes) { try { node.remove(); } catch (e) { } }

return items.length;
return confirmedNodes.size;
}

/**
* Create bulk delete controls in playlist header
*/
ImprovedTube.playlistCreateBulkControls = function () {
let _loadController = null;
if (!this.storage.playlist_bulk_delete_by_progress) return;
if (document.getElementById('it-playlist-cleaner-controls')) return;

Expand Down Expand Up @@ -656,22 +676,32 @@ ImprovedTube.playlistCreateBulkControls = function () {
button.addEventListener('click', async function (e) {
e.preventDefault();
e.stopPropagation();
if (_loadController) {
_loadController.abort();
return;
}
Comment on lines +679 to +682
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The click handler aborts and immediately returns when a run is already in-flight. That means a single re-click only cancels; it does not start a fresh run in the same click (and the button label is 'Cancel', not 'Remove'), which doesn’t match the PR description/test plan. If the intended UX is “re-click to cancel and restart”, adjust the logic to abort the previous controller and immediately proceed with a new controller/run (or update the PR description/test plan to match the actual two-click cancel-then-start behavior).

Copilot uses AI. Check for mistakes.
const threshold = Math.max(0, Math.min(100, parseInt(input.value, 10) || 0));
button.disabled = true;
const thisController = new AbortController();
_loadController = thisController;
const { signal } = thisController;
button.textContent = 'Cancel';
try {
status.textContent = 'Loading…';
await loadAllPlaylistItems(count => {
const timedOut = await loadAllPlaylistItems(count => {
status.textContent = `Loaded ${count}…`;
ImprovedTube.playlistAttachQuickButtons();
});
}, signal);
if (signal.aborted) { status.textContent = 'Cancelled'; return; }
if (timedOut) { status.textContent = 'Timed out — partial list'; return; }
status.textContent = 'Removing…';
const removed = await runRemoval(threshold);
const removed = await runRemoval(threshold, signal);
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cancellation during runRemoval isn’t handled in the UI: if the user clicks Cancel while removal is in progress, runRemoval can return a partial count and the status will still show Removed N (or No matches). Add a signal.aborted check after runRemoval resolves and set a cancellation/partial-completion status instead of reporting success.

Suggested change
const removed = await runRemoval(threshold, signal);
const removed = await runRemoval(threshold, signal);
if (signal.aborted) {
status.textContent = removed ? `Cancelled — removed ${removed}` : 'Cancelled';
return;
}

Copilot uses AI. Check for mistakes.
status.textContent = removed ? `Removed ${removed}` : 'No matches';
} catch (err) {
status.textContent = 'Error';
console.error('[ImprovedTube] Bulk removal error', err);
} finally {
setTimeout(() => { button.disabled = false; }, 200);
if (_loadController === thisController) _loadController = null;
button.textContent = 'Remove';
}
}, true);

Expand Down Expand Up @@ -725,22 +755,32 @@ ImprovedTube.playlistCreateBulkControls = function () {
button.addEventListener('click', async function (e) {
e.preventDefault();
e.stopPropagation();
if (_loadController) {
_loadController.abort();
return;
}
Comment on lines +758 to +761
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same discrepancy in the standard-layout handler: when _loadController exists the handler only aborts and returns, so it won’t restart the operation in the same click. Either change the behavior to match the “cancel & restart” expectation, or align the PR description/test plan with the current two-step UX.

Copilot uses AI. Check for mistakes.
const threshold = Math.max(0, Math.min(100, parseInt(input.value, 10) || 0));
button.disabled = true;
const thisController = new AbortController();
_loadController = thisController;
const { signal } = thisController;
button.textContent = 'Cancel';
try {
status.textContent = 'Loading…';
await loadAllPlaylistItems(count => {
const timedOut = await loadAllPlaylistItems(count => {
status.textContent = `Loaded ${count}…`;
ImprovedTube.playlistAttachQuickButtons();
});
}, signal);
if (signal.aborted) { status.textContent = 'Cancelled'; return; }
if (timedOut) { status.textContent = 'Timed out — partial list'; return; }
status.textContent = 'Removing…';
const removed = await runRemoval(threshold);
const removed = await runRemoval(threshold, signal);
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above for the second bulk-remove button handler: if the abort signal is triggered during removal, the code can still report Removed N even though the run was cancelled mid-way. Check signal.aborted after runRemoval and update status accordingly (e.g., 'Cancelled' / 'Cancelled — removed N so far').

Suggested change
const removed = await runRemoval(threshold, signal);
const removed = await runRemoval(threshold, signal);
if (signal.aborted) {
status.textContent = removed ? `Cancelled — removed ${removed} so far` : 'Cancelled';
return;
}

Copilot uses AI. Check for mistakes.
status.textContent = removed ? `Removed ${removed}` : 'No matches';
} catch (err) {
status.textContent = 'Error';
console.error('[ImprovedTube] Bulk removal error', err);
} finally {
setTimeout(() => { button.disabled = false; }, 200);
if (_loadController === thisController) _loadController = null;
button.textContent = 'Remove';
}
}, true);

Expand Down
Loading