Skip to content

Fix fire-and-forget TriggerSyncAsync: log exceptions instead of silently discarding them#177

Open
davidortinau wants to merge 4 commits into
mainfrom
op-fix-sync-fire-and-forget
Open

Fix fire-and-forget TriggerSyncAsync: log exceptions instead of silently discarding them#177
davidortinau wants to merge 4 commits into
mainfrom
op-fix-sync-fire-and-forget

Conversation

@davidortinau
Copy link
Copy Markdown
Owner

Problem

Every repository in src/SentenceStudio.Shared/Data/ called _syncService?.TriggerSyncAsync().ConfigureAwait(false) as fire-and-forget after SaveChangesAsync(). In .NET 5+, unobserved task exceptions are silently discarded — no crash, no log. .ConfigureAwait(false) on a non-awaited Task is also a no-op. If TriggerSyncAsync threw (connection refused, auth failure, serialization error), the error was lost, the write appeared successful to the caller, and data never synced — with zero indication anything went wrong.

Fix

Added a private void TriggerSync() helper to each affected repository that runs the sync in Task.Run and catches/logs any exception via ILogger:

private void TriggerSync()
{
    if (_syncService is null) return;
    _ = Task.Run(async () =>
    {
        try { await _syncService.TriggerSyncAsync(); }
        catch (Exception ex) { _logger.LogError(ex, "Background sync trigger failed"); }
    });
}

Replaced all 29 silent call sites with TriggerSync().

Changes

File Sites replaced
LearningResourceRepository.cs 12
ScenarioRepository.cs 2
SkillProfileRepository.cs 2
StoryRepository.cs 2
StreamHistoryRepository.cs 2
UserActivityRepository.cs 2
UserProfileRepository.cs 3
VocabularyLearningContextRepository.cs 2
VocabularyProgressRepository.cs 2
Total 29

Verification

  • SentenceStudio.Shared builds clean (0 errors)
  • ✅ 452/452 unit tests pass (dotnet test tests/SentenceStudio.UnitTests)

davidortinau and others added 4 commits April 24, 2026 10:57
- Orchestration log: DX24 vocab crash fix workflow (commit c9b1d0a)
- Session log: ExposureCount NULL→0 backfill, verified on device
- Decision merge: inbox → decisions.md (3 entries: lexical patch, validation strategy, directive)
- Deleted: inbox files + jayne-locale-screenshots (deduped)
- Wash history: append CRITICAL RULE for SQLite migration defense-in-depth (DEFAULT + idempotent backfill)
- No archival needed (no entries >30d old in decisions.md)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Orchestrated multi-agent session for new generic data import feature architecture:

- Zoe (Lead): Full MVP architecture covering UX flows, content detection (heuristic-first,
  AI fallback), data model (no new tables), parsing pipeline, AI integration, service layer,
  scope phasing. Placement revision: separate `/import-content` page (not tab on existing
  `/import`), per Captain directive.

- Wash (Data Layer Scout): Surveyed existing YouTube pipeline pattern, found file import
  UI/service gap, confirmed no schema changes needed, identified dedup inconsistency for
  standardization.

- River (AI Strategy): Designed 5 AI prompt tasks with confidence thresholds, heuristic
  fallbacks, structured DTOs via `SendPrompt<T>`, permissive grading principles.

- Kaylee (UI Scout): Documented form patterns, file picker abstractions, resource lookup
  UI, navigation placement. Recommendations for ContentImportService, InputFile, preview
  table reuse.

- Copilot Directive: Confirmed scope separation — Video Subscriptions (existing) separate
  from generic content import (new) to keep features focused and growth paths independent.

Artifacts:
- 5 orchestration logs (one per agent) in .squad/orchestration-log/
- Session log in .squad/log/2026-04-24T22:31-data-import-architecture-plan.md
- 6 decisions merged into .squad/decisions.md (168KB total)
- All agent histories updated with import-plan context

Next: Implementation team begins ContentImportService + ImportContent.razor page. River
engineers 3 prompt templates (Format Inference, Content Classification, Extraction).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Captain resolved 7 open questions from architecture proposal:

1. Dedup default: Skip and reuse existing (safest, matches YouTube pipeline)
2. Source attribution: Deferred to v2
3. Single-column imports: Use AI to translate on import (new mvp-single-column-translate work item)
4. Mobile clipboard paste: Not needed for MVP
5. Replace-all mode: Deferred to v2
6. Renamed YouTube page: 'Media Import' at /media-import (not 'Video Subscriptions')
7. Separate-page placement: Confirmed — /import-content independent from /media-import

Updated decisions.md with final rulings appended and prior superseded mentions annotated.
All prior architecture content preserved; only updated with Captain's decisions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…() helper

Replace all 29 _syncService?.TriggerSyncAsync().ConfigureAwait(false) call
sites across 9 repository files with a private TriggerSync() helper that
runs the sync in Task.Run and catches/logs any exception via ILogger.

Previously, unobserved task exceptions were silently discarded in .NET 5+.
The .ConfigureAwait(false) on a non-awaited Task was also a no-op.

Affected files:
- LearningResourceRepository.cs (12 sites)
- ScenarioRepository.cs (2 sites)
- SkillProfileRepository.cs (2 sites)
- StoryRepository.cs (2 sites)
- StreamHistoryRepository.cs (2 sites)
- UserActivityRepository.cs (2 sites)
- UserProfileRepository.cs (3 sites)
- VocabularyLearningContextRepository.cs (2 sites)
- VocabularyProgressRepository.cs (2 sites)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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