Skip to content

Fix event loop blocking and add database timeouts#9

Merged
drewburchfield merged 4 commits intomasterfrom
fix/event-loop-blocking-and-timeouts
Apr 3, 2026
Merged

Fix event loop blocking and add database timeouts#9
drewburchfield merged 4 commits intomasterfrom
fix/event-loop-blocking-and-timeouts

Conversation

@drewburchfield
Copy link
Copy Markdown
Owner

@drewburchfield drewburchfield commented Apr 3, 2026

Closes NAS-394

Summary

  • Make embed(), embed_batch(), and embed_with_chunks() async, routed through ThreadPoolExecutor via _call_api_with_timeout()
  • Add _with_timeout() helper to vector_store.py, wrapped 6 database methods with appropriate timeouts (5-30s)
  • Add symlink loop error handling in security_utils.py
  • Replace deprecated asyncio.get_event_loop() with asyncio.get_running_loop()
  • Update all callers (server.py, file_watcher.py, indexer.py, diagnose_vault.py)
  • Update all test mocks from MagicMock to AsyncMock for async methods

Test plan

  • Quality gate review: 4 critical findings (missing async callers) remediated
  • All test mocks updated to AsyncMock
  • Braintrust consultation: aligned on ThreadPoolExecutor approach

Open with Devin

- Make embed_batch() and embed() async, route API calls through
  _call_api_with_timeout() to avoid blocking the event loop with
  synchronous time.sleep() backoff
- Add _with_timeout() helper to VectorStore and wrap all pool
  operations with appropriate timeouts (5s for reads, 10s for
  writes, 30s for batch operations)
- Add error handling for symlink resolution failures in
  security_utils.py path validation
- Replace deprecated asyncio.get_event_loop() with
  asyncio.get_running_loop() in embedder.py and server.py
- Update all callers of embed()/embed_batch() to use await
  (server.py, diagnose_vault.py)
… mocks

- Make embed_with_chunks() async with await on embed() and _call_api_with_timeout()
- Add await to callers in file_watcher.py and indexer.py
- Switch test mocks from MagicMock to AsyncMock for embed/embed_batch/embed_with_chunks
- Add await to integration test embed calls in test_tools.py and test_mcp_tools_integration.py
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

View 6 additional findings in Devin Review.

Open in Devin Review

Comment on lines +53 to +54
problematic_files.append((rel_path, str(e)))
print(f"[{i}/{len(md_files)}] ERROR: {rel_path} - {e}")
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 rel_path used before assignment in outer exception handler

If open(file_path) at line 28 or f.read() at line 29 throws an exception, execution jumps to the outer except Exception at line 52. At that point, rel_path (assigned at line 31) has not been defined yet. On the first loop iteration this causes a NameError, crashing the diagnostic tool. On subsequent iterations, rel_path retains the value from the previous iteration, silently attributing the error to the wrong file.

Suggested change
problematic_files.append((rel_path, str(e)))
print(f"[{i}/{len(md_files)}] ERROR: {rel_path} - {e}")
rel_path = str(file_path.relative_to(vault_path)) if vault_path in file_path.parents or file_path.parent == vault_path else str(file_path)
problematic_files.append((rel_path, str(e)))
print(f"[{i}/{len(md_files)}] ERROR: {rel_path} - {e}")
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@drewburchfield drewburchfield merged commit 4acb642 into master Apr 3, 2026
4 checks passed
@drewburchfield drewburchfield deleted the fix/event-loop-blocking-and-timeouts branch April 3, 2026 19:09
drewburchfield added a commit that referenced this pull request Apr 3, 2026
…ng table

- diagnose_vault.py: compute rel_path before try block to avoid NameError
  when file open/read fails (Devin PR #9)
- schema.sql: move DROP TRIGGER after CREATE TABLE so fresh databases
  don't fail on non-existent table (Devin PR #10)
- vector_store.py: guard DROP TRIGGER migration behind table_exists check
  to prevent initialize() failure on fresh databases (Devin PR #10)
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.

1 participant