fix(taxonomy): reap stalled runs so taxonomy generation can recover (ENG-1924)#109
Conversation
WalkthroughAdds configurable timeout and interval settings for taxonomy run reaping. Implements a repository method that marks stale pending or running runs as failed with error and completion metadata. Wires a cancellable periodic reaper into application startup when taxonomy service configuration is present. Adds persistence tests for stale running and pending runs and verifies fresh runs remain active. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/api/app.go`:
- Around line 805-809: Update runTaxonomyRunReaper to return immediately when
either interval or timeout is zero or negative, before creating the ticker or
calculating cutoff timestamps. This must safely disable the reaper and prevent
invalid configuration from causing a panic or processing active runs
incorrectly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ec32e0f0-58ff-4837-b8bb-3e13c65289a0
📒 Files selected for processing (4)
cmd/api/app.gointernal/config/config.gointernal/repository/taxonomy_repository.gotests/taxonomy_persistence_test.go
BhagyaAmarasinghe
left a comment
There was a problem hiding this comment.
Some comments including AI review
xernobyl
left a comment
There was a problem hiding this comment.
Two low-severity notes, neither blocking — the change is correct and CI is green. Security pass (gitleaks + manual injection/PII/egress check) came back clean.
xernobyl
left a comment
There was a problem hiding this comment.
Thank you for the changes. LGTM.
What does this PR do?
A taxonomy run can be orphaned in
pending/runningif the taxonomy service crashes mid-run or its terminal callback (POST /internal/v1/taxonomy/runs/{id}/failedor the success PUT) is lost. The Hub never transitions the run, so Formbricks Web treats it as "in progress" — it disables Generate and pollsGET /runs/{id}every 5s forever, recoverable only by a manual DB edit.This adds a periodic Hub-side reaper that force-fails runs stalled past a timeout, so polling stops, the failure surfaces, and generation can be retried.
TaxonomyRepository.FailStuckRuns(ctx, olderThan, message, code)— a single bulkUPDATEmarkingpending/runningruns older than the cutoff (COALESCE(started_at, created_at)) asfailed. Idempotent and race-safe: thestatus IN ('pending','running')filter skips runs that finished on their own between sweeps.runTaxonomyRunReaper— a ticker poller mirroring the existingrunRiverQueueDepthPoller, launched in the API process only when the taxonomy service is configured. Cross-tenant, idempotent, safe under multiple replicas.TAXONOMY_STUCK_RUN_TIMEOUT_SECONDS(default 1800) andTAXONOMY_REAPER_INTERVAL_SECONDS(default 60), via the existingDurationSecpattern.Decisions flagged for review:
error_code = internal_errorand the raw message "taxonomy run timed out without completing" (operator-facing; Formbricks Web maps the code to a localized message — ENG-1917). Alternative:service_unavailable(retry-friendlier). Easy to flip.No schema/migration changes (uses existing columns). No API changes.
Closes ENG-1924 · Follow-up to ENG-1213 · pairs with the Web-side wedge fix ENG-1917.
How should this be tested?
TestTaxonomyRepository_FailStuckRuns(intests/) creates a stuckrunningrun (agedstarted_at), a stuckpendingrun (agedcreated_at), and a fresh run, then asserts the reaper fails the two stuck ones and leaves the fresh one. Runs viamake tests(needstest_db) — executed in CI.running; withinTAXONOMY_STUCK_RUN_TIMEOUT_SECONDS + TAXONOMY_REAPER_INTERVAL_SECONDSthe reaper marks itfailed, the UI stops polling, and Generate re-enables.go build ./...,make fmt,make lint(0 issues),go vet ./tests/.Checklist
Required
make buildmake tests(integration tests intests/)make fmtandmake lint; no new warningsgit pull origin mainmigrations/with goose annotations and ranmake migrate-validate— N/A, no schema changeAppreciated
make testsor API contract workflow) — N/A, no API changedocs/if changes were necessarymake tests-coveragefor meaningful logic changes🤖 Generated with Claude Code