Summary
internal/execution/fanout.go:69-74 launches one goroutine per item and has no recover(). The helper is the workhorse for both the per-account fan-out AND the per-rec fan-out inside internal/purchase/execution.go:processPurchaseRecommendations. A panic inside fn (e.g. nil deref on a mocked-but-unexpected provider response, type assertion failure, slice OOB) is fatal:
defer wg.Done() and defer { <-sem }() still fire (so the fan-out helper itself doesn't deadlock)
- The panic propagates up the goroutine stack with no upstream
recover()
- Go runtime crashes the entire process — Lambda invocation terminates abnormally
- Purchase execution row is left at
approved (was set before fan-out kicked off; nothing transitions it to failed or completed)
- CloudWatch may not flush the final log lines before the process dies
- User sees a generic Lambda invocation error, not the actual panic message
This is the silent-failure trap behind issue #632 ("approve strands execution in 'approved' on interrupted sync execution"). One panic anywhere in the purchase chain (cloud SDK, our codec, our parser, mock factory in tests) can strand a real execution.
Audit of all production goroutines
| File:Line |
recover()? |
Risk |
internal/scheduler/scheduler.go:1021 background refresh |
✅ |
safe |
internal/server/app.go:206 migration runner |
✅ |
safe |
internal/api/ri_utilization_cache.go:151 cache refresh |
✅ |
safe |
internal/execution/fanout.go:69 purchase fan-out helper |
❌ |
HIGH |
internal/auth/service_apikeys.go:279 async UpdateLastUsed |
❌ |
LOW (small DB UPDATE) |
internal/api/db_rate_limiter.go:137 async cleanup |
❌ |
LOW |
internal/api/handler_accounts.go:811 token exchange (in-flight only) |
❌ |
LOW (timeout-bounded) |
Fix
Add a recover() deferred guard inside the goroutine in FanOutWithConcurrency that catches the panic, converts it to an error with the panic value + the per-item account ID, and writes it to the per-index results[idx] so the parent aggregator sees a clean r.Err != nil outcome and can record the failure on the execution row:
go func(idx int, accountID string) {
defer wg.Done()
defer func() { <-sem }()
defer func() {
if r := recover(); r != nil {
// Capture the panic + stack so post-mortem doesn't require
// re-running the failure under the debugger. Surface as an
// Err on the result slot so the parent aggregator records
// the failure on the execution row.
buf := make([]byte, 4096)
n := runtime.Stack(buf, false)
logging.Errorf("fan-out goroutine panic (account=%s): %v\n%s", accountID, r, buf[:n])
results[idx] = Result[T]{
AccountID: accountID,
Err: fmt.Errorf("panic during fan-out (account=%s): %v", accountID, r),
}
}
}()
val, err := fn(ctx, accountID)
results[idx] = Result[T]{AccountID: accountID, Value: val, Err: err}
}(i, id)
Optionally also add recover() to the three LOW-risk goroutines in service_apikeys.go, db_rate_limiter.go, handler_accounts.go for completeness — those don't strand execution state but a panic in any of them still kills the Lambda.
Acceptance criteria
Severity
P1 / high. Doesn't match the QA-reported #667 symptom directly (that one IS a context-deadline exceeded with a structured SDK error), but is the same class of bug — a sync purchase failure that leaves execution stranded. Fixing this hardens against the next purchase failure that's a panic rather than a structured error.
Cross-references
Summary
internal/execution/fanout.go:69-74launches one goroutine per item and has norecover(). The helper is the workhorse for both the per-account fan-out AND the per-rec fan-out insideinternal/purchase/execution.go:processPurchaseRecommendations. A panic insidefn(e.g. nil deref on a mocked-but-unexpected provider response, type assertion failure, slice OOB) is fatal:defer wg.Done()anddefer { <-sem }()still fire (so the fan-out helper itself doesn't deadlock)recover()approved(was set before fan-out kicked off; nothing transitions it tofailedorcompleted)This is the silent-failure trap behind issue #632 ("approve strands execution in 'approved' on interrupted sync execution"). One panic anywhere in the purchase chain (cloud SDK, our codec, our parser, mock factory in tests) can strand a real execution.
Audit of all production goroutines
internal/scheduler/scheduler.go:1021background refreshinternal/server/app.go:206migration runnerinternal/api/ri_utilization_cache.go:151cache refreshinternal/execution/fanout.go:69purchase fan-out helperinternal/auth/service_apikeys.go:279async UpdateLastUsedinternal/api/db_rate_limiter.go:137async cleanupinternal/api/handler_accounts.go:811token exchange (in-flight only)Fix
Add a
recover()deferred guard inside the goroutine inFanOutWithConcurrencythat catches the panic, converts it to an error with the panic value + the per-item account ID, and writes it to the per-indexresults[idx]so the parent aggregator sees a cleanr.Err != niloutcome and can record the failure on the execution row:Optionally also add recover() to the three LOW-risk goroutines in
service_apikeys.go,db_rate_limiter.go,handler_accounts.gofor completeness — those don't strand execution state but a panic in any of them still kills the Lambda.Acceptance criteria
FanOutWithConcurrencycatches panics infnand surfaces them asResult.Errinstead of crashing the processinternal/execution/fanout_test.gothat panics fromfnand asserts the result slot hasErrcontaining "panic" + the panic valueexecuteSinglePurchaseshould leave the execution atfailed(notapproved), with the panic visible inexec.ErrorSeverity
P1 / high. Doesn't match the QA-reported #667 symptom directly (that one IS a context-deadline exceeded with a structured SDK error), but is the same class of bug — a sync purchase failure that leaves execution stranded. Fixing this hardens against the next purchase failure that's a panic rather than a structured error.
Cross-references