Skip to content

Feature: Add AbortController cleanup for async operations in modals #60

Description

@ojfbot

Summary

Add proper cleanup for async operations in document modal components to prevent memory leaks from state updates on unmounted components.

Problem

Both DocumentChatModal and DocumentPreviewModal have useEffect hooks that trigger async operations (loadDocumentContext, loadPreview) but lack cleanup handlers. If a user closes the modal while data is loading, the component may attempt to update state after unmounting, causing memory leaks and React warnings.

Affected Files

  • packages/browser-app/src/components/DocumentChatModal.tsx (lines 69-71)
  • packages/browser-app/src/components/DocumentPreviewModal.tsx (lines 80-82)

Proposed Solution

Implement AbortController pattern for async cleanup:

useEffect(() => {
  const abortController = new AbortController()
  
  const loadData = async () => {
    try {
      setLoading(true)
      setError(null)
      
      // Pass signal to API calls
      const data = await bioFilesApi.getSummary(fileId, { 
        signal: abortController.signal 
      })
      
      // Check if aborted before state updates
      if (!abortController.signal.aborted) {
        setMessages([introMessage])
      }
    } catch (err) {
      if (err.name === 'AbortError') return // Ignore abort errors
      if (!abortController.signal.aborted) {
        setError(err instanceof Error ? err.message : 'Failed to load')
      }
    } finally {
      if (!abortController.signal.aborted) {
        setLoading(false)
      }
    }
  }
  
  loadData()
  
  return () => {
    abortController.abort()
  }
}, [fileId])

Implementation Checklist

  • Add AbortController to DocumentChatModal useEffect
  • Add AbortController to DocumentPreviewModal useEffect
  • Update bioFilesApi methods to accept signal parameter
  • Update fetch calls in bioFilesApi to use AbortSignal
  • Add proper error handling for AbortError
  • Test modal unmounting during loading state
  • Verify no React warnings about unmounted state updates

Additional Context

This is part of PR #57 review feedback. Related to proper resource cleanup and preventing memory leaks in React components.

Priority

Medium - Prevents memory leaks but doesn't affect core functionality

Parent Issue

Part of PR #57 review feedback: #57

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestfeatureMajor new feature or capability

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions