Skip to content

Add retry limits, error boundaries, and fix async init race in UploadService#60

Draft
Copilot wants to merge 2 commits intomainfrom
copilot/add-retry-limits-error-boundaries
Draft

Add retry limits, error boundaries, and fix async init race in UploadService#60
Copilot wants to merge 2 commits intomainfrom
copilot/add-retry-limits-error-boundaries

Conversation

Copy link
Copy Markdown

Copilot AI commented Mar 19, 2026

Three reliability gaps: failed uploads retry indefinitely, uncaught render errors crash the UI to a blank screen, and UploadService constructor fires restoreQueue() fire-and-forget — allowing addToQueue() calls to race against queue restoration.

Changes

UploadService: retry limit

  • Added MAX_RETRIES = 3 static constant
  • handleUploadError() increments asset.retryCount on each failure; after >3 retries the asset is marked permanently_failed and logged — no further automatic re-queuing
  • retryFailedUploads() already filters to status === 'failed', so permanently failed assets are excluded automatically

UploadService: async init race

  • Constructor now assigns this.readyPromise = this.restoreQueue() instead of discarding the promise
  • addToQueue() and addMultipleToQueue() both await this.readyPromise before mutating the queue
private readyPromise: Promise<void>;

constructor(...) {
  this.readyPromise = this.restoreQueue();
}

async addToQueue(asset: Asset, isManualRetry = false): Promise<void> {
  await this.readyPromise; // queue fully restored before any writes
  ...
}

Asset type (IndexedDBService)

  • Added retryCount?: number field
  • Added 'permanently_failed' to the status union

React error boundaries

  • New shared src/components/ErrorBoundary.tsx — catches render errors and shows a reload prompt (window.location.reload()) instead of a blank screen
  • Applied to root render in both popup.tsx and options.tsx
Original prompt

This section details on the original issue you should resolve

<issue_title>[Feature][Medium] Add retry limits, error boundaries, and fix async initialization race</issue_title>
<issue_description>## Summary

Three medium-priority feature improvements discovered during deep audit of the main branch (commit cc3b576):

1. No Maximum Retry Limit for Failed Uploads (UploadService.ts:207-229)

When an upload fails, the asset is re-queued via handleUploadError() but there is no maximum retry counter. If an asset consistently fails (e.g., corrupted data, server-side rejection), it will be retried indefinitely every time the queue processes. This wastes bandwidth, battery, and API quota.

File: src/services/UploadService.tsprocessQueue() at line 207 and handleUploadError() flow

Suggested fix: Add a retryCount field to the Asset type and increment on each failure. After N retries (e.g., 3), mark the asset as permanently_failed and notify the user instead of re-queuing.

2. Missing React Error Boundary in Popup and Options Pages (popup.tsx, options.tsx)

Neither the popup nor the options page wraps its component tree in a React Error Boundary. If any component throws during render (e.g., corrupted storage data, unexpected API response shape), the entire UI crashes to a blank white screen with no recovery path. The user must close and reopen the popup.

Files:

  • src/popup/popup.tsx — root createRoot(document.getElementById('root')!).render(<App />)
  • src/options/options.tsx — root createRoot(document.getElementById('root')!).render(<Options />)

Suggested fix: Create a generic ErrorBoundary component and wrap the root:

class ErrorBoundary extends React.Component {
  state = { hasError: false };
  static getDerivedStateFromError() { return { hasError: true }; }
  render() {
    if (this.state.hasError) {
      return <div className="error-fallback">
        <p>Something went wrong.</p>
        <button onClick={() => this.setState({ hasError: false })}>Retry</button>
      </div>;
    }
    return this.props.children;
  }
}

createRoot(root).render(<ErrorBoundary><App /></ErrorBoundary>);

3. Constructor Fires Async restoreQueue() Without Awaiting (UploadService.ts:29-35)

The UploadService constructor calls this.restoreQueue() (an async method) without awaiting the result. Since constructors cannot be async, the queue restoration runs as a fire-and-forget promise. If any code calls addToQueue() before restoration completes, the restored items may be overwritten or duplicated.

File: src/services/UploadService.ts lines 29-35

constructor(
  private apiClient: ApiClient,
  private assetStorage = indexedDBService,
  private metadataStorage = storageService
) {
  this.restoreQueue();  // async but not awaited
}

Suggested fix: Use a lazy initialization pattern with a ready promise:

private readyPromise: Promise<void>;

constructor(...) {
  this.readyPromise = this.restoreQueue();
}

async addToQueue(asset: Asset): Promise<void> {
  await this.readyPromise;  // Ensure queue is restored first
  // ... existing logic
}

Impact

  • Infinite retries waste resources and degrade user experience for permanently failed uploads
  • Missing error boundaries cause silent white-screen crashes with no recovery
  • Unguarded async constructor can cause queue data loss or duplication</issue_description>

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


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: numbers-official <181934381+numbers-official@users.noreply.github.com>
Copilot AI changed the title [WIP] Add retry limits and error boundaries with async initialization fix Add retry limits, error boundaries, and fix async init race in UploadService Mar 19, 2026
Copilot AI requested a review from numbers-official March 19, 2026 21:16
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][Medium] Add retry limits, error boundaries, and fix async initialization race

2 participants