Files
insta-recipe/docs/plans/FixServiceWorkerAndQueueStreamBugs.md
Giancarmine Salucci 93aa25a31c fix: resolve critical app functionality issues
Complete implementation of fixes for queue processing, SSE connection display, service worker installation, and failing tests.

Key Changes:
- Fix queue processor startup with proper import and subscription mechanism
- Implement centralized API error handling middleware for proper HTTP status codes
- Enhance service worker configuration for PWA compliance and reliability
- Fix SSE connection display with reactive state management
- Add comprehensive test coverage and health check endpoints

Results:
- All 169 tests now passing (previously 16 failing)
- Queue items process immediately from pending to success/error states
- Real-time SSE connection status with auto-reconnection logic
- Proper PWA functionality with working service worker registration
- API endpoints return correct HTTP status codes (400/404/409) instead of 500 errors

This resolves the critical issues preventing core app functionality and enables proper production deployment.
2025-12-22 04:27:59 +01:00

22 KiB

Execution Plan: Fix Service Worker and Queue Stream Bugs

Created: 2025-12-22
Status: Planning
Priority: Critical - Production blocking bugs

Executive Summary

Multiple critical bugs are preventing the application from functioning correctly:

  1. Service Worker Evaluation Error: Browser console shows "ServiceWorker script threw an exception during script evaluation"
  2. Stream Controller Errors: Server logs show repeated "ERR_INVALID_STATE: Controller is already closed" errors
  3. Frontend Display Bug: Queue items not rendering in UI despite counters updating
  4. Push Notifications Broken: Service worker not responding to push notification requests

These bugs are interconnected - the service worker failure blocks push notifications, the stream controller errors spam server logs, and the frontend bug prevents users from seeing any queue items.

Root Cause Analysis

Bug 1: Service Worker Script Evaluation Error

Location: src/service-worker.ts
Symptom: Browser console error during service worker registration
Root Cause:

  • Service worker script failing during initial evaluation/parsing
  • Potential causes:
    • Workbox imports not being resolved correctly in built output
    • TypeScript type references in compiled JavaScript
    • Missing error handling causing uncaught exceptions during initialization
    • Undefined globals or browser APIs called at top level

Impact: High - Blocks PWA functionality and push notifications

Bug 2: Stream Controller Already Closed Errors

Location: src/routes/api/queue/stream/+server.ts (lines 75, 100)
Symptom: TypeError [ERR_INVALID_STATE]: Invalid state: Controller is already closed
Root Cause:

  • ReadableStreamController doesn't track closed state
  • QueueManager subscribers continue to call enqueue after client disconnects
  • Keep-alive interval continues after stream is closed
  • Multiple cleanup handlers don't coordinate properly
  • No defensive checks before enqueue operations

Impact: High - Spams server logs, prevents proper stream cleanup

Bug 3: Frontend Queue Items Not Displaying

Location: src/routes/+page.svelte (line 28-32)
Symptom: Queue counters update but no cards are rendered
Root Cause:

  • Incorrect Svelte 5 runes syntax
  • Current code: let filteredItems = $derived(() => {...})
  • This creates a derived value that IS a function, not the result of calling it
  • Template tries to iterate over a function instead of an array
  • Should use: $derived.by(() => {...}) to execute and derive the result

Impact: Critical - Users cannot see any queue items

Bug 4: Push Notifications Not Working

Location: Service worker registration and push handlers
Symptom: Service worker not responding to push notification requests
Root Cause:

  • Dependent on Bug 1 - if service worker fails to register, no push handlers are available
  • Service worker message handlers not being registered
  • Potential registration timing issues

Impact: High - No real-time notifications for users

Dependencies and Constraints

Technical Dependencies

  • Svelte 5 with runes syntax
  • SvelteKit SSR/SSG architecture
  • Vite + vite-plugin-pwa for PWA functionality
  • Workbox for service worker precaching
  • Server-Sent Events (SSE) for queue updates
  • ReadableStream API for SSE implementation

Constraints

  • Must maintain SSR compatibility (no browser-only code in server context)
  • Must properly clean up resources (event listeners, intervals, subscriptions)
  • Must handle client disconnections gracefully
  • Service worker must work in both development and production modes
  • Cannot break existing PWA functionality (offline support, precaching)

Inter-bug Dependencies

Bug 1 (Service Worker) ──blocks──> Bug 4 (Push Notifications)
                                           
Bug 2 (Stream Controller) ──independent──> Can be fixed in parallel
                                           
Bug 3 (Frontend Display) ──independent──> Can be fixed in parallel

Story Breakdown

Story 1: Fix Service Worker Evaluation Error

Priority: Critical
Dependencies: None
Estimated Effort: Medium

Objective: Resolve service worker script evaluation error to enable PWA functionality and push notifications.

Tasks:

  1. Add comprehensive error handling to service worker initialization
  2. Wrap workbox calls in try-catch blocks
  3. Add fallback behavior for missing workbox manifest
  4. Verify TypeScript compilation produces valid service worker code
  5. Add console logging for debugging service worker lifecycle
  6. Test service worker registration in browser
  7. Verify workbox precaching works correctly

Acceptance Criteria:

  • Service worker registers successfully without errors
  • Browser console shows no evaluation errors
  • Workbox precaching initializes correctly
  • Service worker enters 'activated' state
  • Push notification handlers are registered
  • Notification click handlers work correctly
  • Service worker survives page reloads
  • PWA manifest is served correctly

Implementation Details:

File: src/service-worker.ts

Add error handling wrapper:

/// <reference types="vite/client" />
/// <reference lib="webworker" />

import { cleanupOutdatedCaches, createHandlerBoundToURL, precacheAndRoute } from 'workbox-precaching';
import { NavigationRoute, registerRoute } from 'workbox-routing';

declare let self: ServiceWorkerGlobalScope;

console.log('[SW] Service worker script loading...');

// Wrap workbox initialization in try-catch
try {
  console.log('[SW] Initializing workbox...');
  
  // Check if manifest exists
  if (self.__WB_MANIFEST) {
    console.log('[SW] Workbox manifest found, precaching...');
    precacheAndRoute(self.__WB_MANIFEST);
    cleanupOutdatedCaches();
  } else {
    console.warn('[SW] Workbox manifest not found, skipping precaching');
  }
  
  // Handle navigation requests
  const handler = createHandlerBoundToURL('/');
  const navigationRoute = new NavigationRoute(handler, {
    denylist: [/^\/api/]
  });
  registerRoute(navigationRoute);
  
  console.log('[SW] Workbox initialized successfully');
} catch (error) {
  console.error('[SW] Error initializing workbox:', error);
  // Continue with service worker registration even if workbox fails
  // This allows push notifications and other features to still work
}

// Rest of service worker code (push notifications, etc.)
// ... (existing code continues)

Testing Strategy:

  1. Clear service worker cache in browser DevTools
  2. Hard reload page (Ctrl+Shift+R)
  3. Check Application > Service Workers in DevTools
  4. Verify service worker status is "activated"
  5. Check console for successful initialization messages
  6. Test offline functionality
  7. Test push notification registration

Story 2: Fix Stream Controller Closed State Errors

Priority: Critical
Dependencies: None
Estimated Effort: Medium

Objective: Prevent "Controller is already closed" errors by properly tracking stream state and coordinating cleanup.

Tasks:

  1. Add closed state tracking flag to stream controller
  2. Check state before all enqueue operations
  3. Consolidate cleanup logic into single function
  4. Properly unsubscribe from QueueManager on disconnect
  5. Clear keep-alive interval when controller closes
  6. Add defensive error handling around enqueue calls
  7. Test client disconnect scenarios

Acceptance Criteria:

  • No "ERR_INVALID_STATE" errors in server logs
  • Stream closes cleanly when client disconnects
  • QueueManager subscriptions are properly cleaned up
  • Keep-alive interval is cleared on disconnect
  • Multiple clients can connect/disconnect without errors
  • Stream handles rapid connect/disconnect cycles
  • Server logs show clean connection/disconnection messages

Implementation Details:

File: src/routes/api/queue/stream/+server.ts

Replace the entire GET handler with fixed implementation:

export const GET: RequestHandler = async ({ url, request }) => {
  const searchParams = url.searchParams;
  const itemIdFilter = searchParams.get('id');
  const statusFilter = searchParams.get('status');
  
  // Validate status filter if provided
  const validStatuses = ['pending', 'in_progress', 'success', 'unhealthy', 'error'];
  if (statusFilter && !validStatuses.includes(statusFilter)) {
    return new Response(`Invalid status filter. Must be one of: ${validStatuses.join(', ')}`, {
      status: 400,
      headers: { 'Content-Type': 'text/plain' }
    });
  }
  
  // Validate item ID filter if provided
  if (itemIdFilter) {
    const uuidPattern = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
    if (!uuidPattern.test(itemIdFilter)) {
      return new Response('Invalid queue item ID format', {
        status: 400,
        headers: { 'Content-Type': 'text/plain' }
      });
    }
  }
  
  // Track stream state
  let isClosed = false;
  let unsubscribe: (() => void) | null = null;
  let keepAliveInterval: NodeJS.Timeout | null = null;
  
  // Unified cleanup function
  const cleanup = () => {
    if (isClosed) return; // Prevent double cleanup
    isClosed = true;
    
    console.log('[SSE] Cleaning up stream connection');
    
    // Unsubscribe from queue updates
    if (unsubscribe) {
      unsubscribe();
      unsubscribe = null;
    }
    
    // Clear keep-alive interval
    if (keepAliveInterval) {
      clearInterval(keepAliveInterval);
      keepAliveInterval = null;
    }
  };
  
  // Safe enqueue helper
  const safeEnqueue = (controller: ReadableStreamDefaultController, message: string) => {
    if (isClosed) {
      return false; // Stream already closed
    }
    
    try {
      controller.enqueue(new TextEncoder().encode(message));
      return true;
    } catch (error) {
      // Controller closed or errored
      console.error('[SSE] Error enqueueing message:', error);
      cleanup();
      return false;
    }
  };
  
  // Create SSE response stream
  const stream = new ReadableStream({
    start(controller) {
      console.log('[SSE] Stream started');
      
      // Send initial connection message
      const connectionMsg = `event: connection\ndata: {"type":"connection","timestamp":"${new Date().toISOString()}","message":"Connected to queue stream"}\n\n`;
      if (!safeEnqueue(controller, connectionMsg)) {
        return;
      }
      
      // Send current queue state as initial data
      try {
        const currentItems = queueManager.getAll();
        let filteredItems = currentItems;
        
        // Apply filters
        if (itemIdFilter) {
          filteredItems = currentItems.filter(item => item.id === itemIdFilter);
        }
        if (statusFilter) {
          filteredItems = filteredItems.filter(item => item.status === statusFilter);
        }
        
        // Send initial state for each matching item
        for (const item of filteredItems) {
          if (isClosed) break;
          
          const update: QueueStatusUpdate = {
            type: 'status_change',
            itemId: item.id,
            status: item.status,
            timestamp: new Date().toISOString(),
            url: item.url,
            progress: item.phases,
            results: item.results,
            error: item.error
          };
          
          const sseMessage = `event: queue-update\ndata: ${JSON.stringify(update)}\n\n`;
          if (!safeEnqueue(controller, sseMessage)) {
            break;
          }
        }
      } catch (error) {
        console.error('[SSE] Error sending initial queue state:', error);
      }
      
      // Subscribe to queue updates
      unsubscribe = queueManager.subscribe((update) => {
        if (isClosed) return; // Don't process if already closed
        
        // Apply filters
        let shouldSend = true;
        
        if (itemIdFilter && update.itemId !== itemIdFilter) {
          shouldSend = false;
        }
        
        if (statusFilter && update.status !== statusFilter) {
          shouldSend = false;
        }
        
        if (shouldSend) {
          const sseMessage = `event: queue-update\ndata: ${JSON.stringify(update)}\n\n`;
          safeEnqueue(controller, sseMessage);
        }
      });
      
      // Keep-alive ping every 30 seconds
      keepAliveInterval = setInterval(() => {
        if (isClosed) {
          if (keepAliveInterval) {
            clearInterval(keepAliveInterval);
            keepAliveInterval = null;
          }
          return;
        }
        
        const pingMsg = `event: ping\ndata: {"type":"ping","timestamp":"${new Date().toISOString()}"}\n\n`;
        if (!safeEnqueue(controller, pingMsg)) {
          if (keepAliveInterval) {
            clearInterval(keepAliveInterval);
            keepAliveInterval = null;
          }
        }
      }, 30000);
      
      // Handle client disconnect
      request.signal.addEventListener('abort', () => {
        console.log('[SSE] Client disconnected (abort signal)');
        cleanup();
        
        // Try to send disconnect message (may fail if already closed)
        const disconnectMsg = `event: disconnect\ndata: {"type":"disconnect","timestamp":"${new Date().toISOString()}","message":"Connection closed"}\n\n`;
        safeEnqueue(controller, disconnectMsg);
        
        // Close the controller
        try {
          controller.close();
        } catch (error) {
          // Already closed, ignore
        }
      });
    },
    
    cancel() {
      // This is called when the stream is cancelled by the client
      console.log('[SSE] Stream cancelled by client');
      cleanup();
    }
  });
  
  return new Response(stream, {
    status: 200,
    headers: {
      'Content-Type': 'text/event-stream',
      'Cache-Control': 'no-cache',
      'Connection': 'keep-alive',
      'Access-Control-Allow-Origin': '*',
      'Access-Control-Allow-Headers': 'Cache-Control',
      'Access-Control-Expose-Headers': 'Content-Type'
    }
  });
};

Testing Strategy:

  1. Start dev server and open browser
  2. Monitor server console for SSE log messages
  3. Connect to queue stream
  4. Add queue items and verify updates received
  5. Close browser tab and verify clean disconnection message
  6. Reconnect and verify no errors in server logs
  7. Test with multiple concurrent clients
  8. Test rapid connect/disconnect cycles
  9. Verify no "Controller is already closed" errors

Story 3: Fix Frontend Queue Items Display

Priority: Critical
Dependencies: None
Estimated Effort: Small

Objective: Fix Svelte 5 runes syntax to properly derive filtered items array so queue cards render correctly.

Tasks:

  1. Change $derived to $derived.by for filteredItems
  2. Verify template renders items correctly
  3. Test filter switching
  4. Test SSE updates trigger re-renders
  5. Verify counters match displayed items

Acceptance Criteria:

  • Queue items cards are displayed when items exist
  • Filtering works correctly for all filter options
  • Item counters match displayed items
  • Real-time updates show new cards
  • Highlighted items display correctly
  • No console errors related to iteration
  • Empty state shows when no items match filter

Implementation Details:

File: src/routes/+page.svelte

Change line 28-32 from:

// WRONG - creates a derived that IS a function
let filteredItems = $derived(() => {
  if (filter === 'all') return items;
  if (filter === 'error') return items.filter(item => item.status === 'error' || item.status === 'unhealthy');
  return items.filter(item => item.status === filter);
});

To:

// CORRECT - executes function and derives the result
let filteredItems = $derived.by(() => {
  if (filter === 'all') return items;
  if (filter === 'error') return items.filter(item => item.status === 'error' || item.status === 'unhealthy');
  return items.filter(item => item.status === filter);
});

Explanation:

  • $derived(() => {...}) - Creates a derived value that IS the function itself
  • $derived.by(() => {...}) - Executes the function and uses its return value as the derived value
  • The template needs an array to iterate over, not a function
  • This is the correct Svelte 5 runes pattern for derived values that need computation

Testing Strategy:

  1. Save the file and verify hot reload works
  2. Check that queue items cards are now visible
  3. Test each filter option (All, Pending, Processing, Complete, Failed)
  4. Verify item counts in filter tabs match displayed cards
  5. Add a new queue item and verify it appears
  6. Test highlighting when redirected from share page
  7. Verify empty state displays correctly when filter returns no items

Story 4: Verify Push Notifications Work

Priority: High
Dependencies: Story 1 (Service Worker Fix)
Estimated Effort: Small

Objective: Verify push notifications work correctly after service worker is fixed.

Tasks:

  1. Wait for Story 1 completion
  2. Test service worker message handling
  3. Test push notification permission request
  4. Test notification display
  5. Test notification click actions
  6. Verify notification data payload
  7. Test background sync for retries

Acceptance Criteria:

  • Push notification permission request dialog appears
  • Permission can be granted successfully
  • Service worker receives push events
  • Notifications display with correct title and body
  • Notification icons and badges display correctly
  • Clicking notification opens app to correct page
  • Notification actions (View, Retry) work correctly
  • Service worker message handler responds

Implementation Details:

No code changes needed if Story 1 is implemented correctly. This is a verification story.

Testing Strategy:

  1. Clear service worker and reload page
  2. Verify service worker registers successfully (from Story 1)
  3. Click "Enable Notifications" in NotificationSettings component
  4. Grant permission in browser dialog
  5. Trigger a queue item to complete
  6. Verify push notification appears with correct data
  7. Click notification and verify app opens to correct page
  8. Test "View Recipe" and "Retry" actions
  9. Verify service worker console logs show message handling

Debugging Steps if Issues Persist:

  1. Check browser console for service worker errors
  2. Verify push subscription created successfully
  3. Check Application > Service Workers in DevTools
  4. Verify notification permission is "granted"
  5. Check service worker console (separate console in DevTools)
  6. Verify push event listeners are registered
  7. Test with simple test notification first

Technical Implementation Notes

Svelte 5 Runes Reference

  • $state<T>() - Reactive state variable
  • $derived - Simple derived value (for expressions)
  • $derived.by(() => {...}) - Derived value with computation function
  • Template reactivity works with all runes automatically

Service Worker Best Practices

  • Always wrap workbox initialization in try-catch
  • Log all lifecycle events for debugging
  • Handle missing manifest gracefully
  • Use proper TypeScript types with /// <reference> directives
  • Test in both development and production modes

ReadableStream Cleanup Pattern

let isClosed = false;

const cleanup = () => {
  if (isClosed) return;
  isClosed = true;
  // ... cleanup logic
};

const safeEnqueue = (controller, message) => {
  if (isClosed) return false;
  try {
    controller.enqueue(message);
    return true;
  } catch (error) {
    cleanup();
    return false;
  }
};

SSE Best Practices

  • Always track connection state
  • Implement unified cleanup function
  • Use abort signal for client disconnect detection
  • Wrap enqueue in try-catch
  • Clear all intervals/timers on disconnect
  • Log connection/disconnection for debugging

Testing Checklist

Service Worker Tests

  • Service worker registers without errors
  • Workbox precaching works
  • Navigation routing works
  • Service worker survives page reloads
  • Service worker updates correctly
  • Offline mode works

Stream Controller Tests

  • Stream connects successfully
  • Initial queue state sent correctly
  • Real-time updates received
  • Client disconnect handled cleanly
  • No errors in server logs
  • Multiple concurrent connections work
  • Rapid connect/disconnect doesn't cause errors
  • Keep-alive pings sent correctly
  • Filters work correctly (id, status)

Frontend Display Tests

  • Queue items cards display
  • All filters work correctly
  • Counters match displayed items
  • Real-time updates show new cards
  • Highlighting works
  • Empty states display correctly
  • Retry/Remove actions work
  • Results display correctly

Push Notification Tests

  • Permission request dialog appears
  • Permission can be granted
  • Notifications display correctly
  • Notification click actions work
  • Service worker receives messages
  • Background sync works

Rollback Plan

If any story causes issues:

  1. Revert Git Commit

    git revert <commit-hash>
    
  2. Restart Dev Server

    npm run dev
    
  3. Clear Service Worker Cache

    • Open DevTools > Application > Service Workers
    • Click "Unregister"
    • Hard reload (Ctrl+Shift+R)
  4. Clear Browser Storage

    • DevTools > Application > Clear Storage
    • Check all boxes
    • Click "Clear site data"

Success Metrics

  • Zero service worker evaluation errors in browser console
  • Zero "Controller is already closed" errors in server logs
  • Queue items display correctly with real-time updates
  • Push notifications work end-to-end
  • All existing functionality continues to work
  • No new errors or warnings introduced
  • Performance remains unchanged

Documentation Updates

After completion:

  1. Update README with troubleshooting section for service worker
  2. Document Svelte 5 runes patterns used in project
  3. Add SSE stream implementation notes to API.md
  4. Document push notification setup in TESTING.md

Dependencies Graph

graph TD
    A[Story 1: Fix Service Worker] --> D[Story 4: Verify Push Notifications]
    B[Story 2: Fix Stream Controller] --> E[Complete]
    C[Story 3: Fix Frontend Display] --> E
    D --> E

Estimated Timeline

  • Story 1: 2-3 hours (including testing)
  • Story 2: 2-3 hours (including testing)
  • Story 3: 30 minutes (simple fix)
  • Story 4: 1 hour (verification only)

Total: 6-8 hours

Notes

  • All bugs are independent except Story 4 depends on Story 1
  • Stories 2 and 3 can be implemented in parallel
  • Each story has clear acceptance criteria for verification
  • Comprehensive testing strategy for each story
  • Rollback plan available if issues arise