Skip to content

Fix upload queue asset loss on failure due to premature shift()#53

Draft
Copilot wants to merge 2 commits intomainfrom
copilot/fix-upload-queue-asset-loss
Draft

Fix upload queue asset loss on failure due to premature shift()#53
Copilot wants to merge 2 commits intomainfrom
copilot/fix-upload-queue-asset-loss

Conversation

Copy link
Copy Markdown

Copilot AI commented Mar 18, 2026

processQueue() called shift() before the upload attempt, so on failure the asset was already removed from the in-memory queue before saveQueue() ran — silently dropping it from the persisted queue and making it unrecoverable across service worker restarts.

Changes

processQueue() — fix premature removal

  • Replaced shift() with a non-destructive peek at uploadQueue[0]
  • Success path: shift() removes the asset, then saveQueue()
  • Failure path: handleUploadError() marks asset failed in IndexedDB, then splice(0,1) + push() moves it to the end of the queue before saveQueue() — preserving the asset ID in persisted storage and allowing other assets to be processed before retry
  • saveQueue() consolidated into a finally block
// Before
const asset = this.uploadQueue.shift(); // removed before upload even starts
try {
  await this.uploadAsset(asset);
} catch (error) {
  await this.handleUploadError(asset, error);
}
await this.saveQueue(); // queue already missing the failed asset

// After
const asset = this.uploadQueue[0]; // peek only
try {
  await this.uploadAsset(asset);
  this.uploadQueue.shift(); // only removed on success
} catch (error) {
  await this.handleUploadError(asset, error);
  this.uploadQueue.push(this.uploadQueue.splice(0, 1)[0]); // move to end for retry
} finally {
  await this.saveQueue(); // always persists current queue state
}

restoreQueue() — recover assets lost by the old bug

Scans IndexedDB for assets with status === 'failed' that are absent from the persisted queue IDs, and re-adds them. Provides backward-compatible recovery for assets previously dropped by the premature shift() across service worker restarts.

Original prompt

This section details on the original issue you should resolve

<issue_title>[Feature][High] Fix upload queue asset loss on failure due to premature shift()</issue_title>
<issue_description>## Summary

The upload queue silently loses assets when uploads fail, making them unrecoverable without manual intervention.

Problem

File: src/services/UploadService.ts (lines 207-229)

In processQueue(), this.uploadQueue.shift() removes the asset from the in-memory queue before the upload attempt (line 213). If the upload fails:

  1. handleUploadError updates the asset status to failed in IndexedDB
  2. But the asset has already been removed from this.uploadQueue
  3. saveQueue() at line 227 persists the queue without the failed asset's ID
  4. If the service worker restarts, the failed asset is not in the persisted queue

The retryFailedUploads() method (line 491) scans IndexedDB for failed assets, but it is never called automatically.

Impact

Failed uploads silently fall out of the queue. On transient network errors, assets can be permanently lost from the queue unless the user manually discovers and retries them. This is especially problematic for:

  • Intermittent network connectivity
  • Server-side rate limiting (429 responses)
  • Temporary API outages

Suggested Fix

Do not shift() until upload succeeds:

async processQueue(): Promise<void> {
  // ...
  const asset = this.uploadQueue[0];
  if (!asset) { this.isUploading = false; return; }
  
  try {
    await this.uploadAsset(asset);
    this.uploadQueue.shift(); // Only remove on success
    await this.saveQueue();
  } catch (error) {
    // Keep in queue for automatic retry, or move to end
    const failedAsset = this.uploadQueue.shift();
    await this.handleUploadError(failedAsset, error);
    // Optionally re-add to end for retry: this.uploadQueue.push(failedAsset);
    await this.saveQueue();
  }
}

Alternatively, call retryFailedUploads() automatically on service worker startup or on a periodic timer.

Note

Issue #22 covers "Add IndexedDB recovery, upload retry with backoff" as a feature enhancement. This finding identifies a correctness bug in the current queue implementation that causes silent data loss, which should be fixed independently of retry/backoff improvements.</issue_description>

Comments on the Issue (you are @copilot in this section)


📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.

Co-authored-by: numbers-official <181934381+numbers-official@users.noreply.github.com>
Copilot AI changed the title [WIP] [Feature] Fix upload queue asset loss on failure due to premature shift Fix upload queue asset loss on failure due to premature shift() Mar 18, 2026
Copilot AI requested a review from numbers-official March 18, 2026 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature][High] Fix upload queue asset loss on failure due to premature shift()

2 participants