Skip to content

[Feature][Medium] Add retry limits, error boundaries, and fix async initialization race #57

@numbers-official

Description

@numbers-official

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

Metadata

Metadata

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions