fix(cli): add recover() guard to queryRDSInstancesInRegions worker goroutine

[cristim]

Summary

• cmd/multi_service_engine_versions.go:112 spawns one goroutine per AWS region that calls queryRDSInstancesInRegion (map writes + AWS SDK calls, both can panic). A panic in one region's worker crashed the whole CLI process, silently aborting all remaining regions.
• Add a deferred recover() that logs the panic with region context (log.Printf("ERROR: panic in region worker (region=%s): %v\n%s", regionName, r, buf[:n])), then allows the goroutine to exit cleanly. The other workers continue and wg.Wait() collects their results.
• Mirrors the pattern applied to Lambda fire-and-forget goroutines in PR chore(reliability): recover() in fire-and-forget goroutines (closes #672) #859 (issue chore(reliability): add recover() to non-fan-out fire-and-forget goroutines (auth, rate-limiter, GCP token-exchange) #672).

Closes #997

Test plan

• go build ./cmd/... succeeds.
• go test ./cmd/... passes (all existing tests green).
• Pattern mirrors internal/execution/fanout.go recover() guard which has a unit test asserting panic isolation (TestFanOut_PanicInFn).

Labels (mirrored from issue #997)

priority/p3 severity/low urgency/eventually impact/internal effort/xs type/bug triaged

---

Generated by Claude Code

[cristim]

@coderabbitai review

---

Generated by Claude Code

[coderabbitai]

Warning

Review limit reached

@cristim, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 49 minutes and 37 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 90e29ed4-1f5b-4615-a3c6-dd5fb7b63c16

📥 Commits

Reviewing files that changed from the base of the PR and between edcf9dd and 412aa16.

📒 Files selected for processing (1)

• cmd/multi_service_engine_versions.go

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/997-cli-goroutine-recover

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get PR details
gh pr view 1111 --repo LeanerCloud/CUDly

echo "---DIFF---"
gh pr diff 1111 --repo LeanerCloud/CUDly

Length of output: 2592

---

🏁 Script executed:

#!/bin/bash
# Read the fanout.go file for pattern comparison
fd -e go fanout.go | xargs cat

echo "---FULL GOROUTINE CONTEXT---"
# Read the full file
fd -e go multi_service_engine_versions.go | xargs cat

echo "---ISSUE 997---"
gh issue view 997 --repo LeanerCloud/CUDly 2>/dev/null || echo "Issue not accessible"

Length of output: 22209

---

---

@cristim: Reviewed the change against internal/execution/fanout.go and the full function context. Everything looks correct.

Defer order (the critical correctness question):

defer wg.Done()           // registered 1st → runs last
defer func() { <-sem }() // registered 2nd → runs 2nd
defer func() { recover() ... }() // registered 3rd → runs FIRST (LIFO)

The recover guard executes before the cleanup defers, catches the panic, then <-sem and wg.Done() run normally — exactly right.

Consistency with fanout.go:

Aspect	fanout.go	this PR
Defer positioning	last-registered (runs first)	✅ same
Stack buf size	4096	✅ same
runtime.Stack(buf, false)	✓	✅ same
Logger	logging.Errorf	log.Printf("ERROR: ...") (cmd/ doesn't import logging pkg) ✅
Panic surfaced as error	yes (results[idx].Err)	N/A — queryRDSInstancesInRegion is void; no result slot exists

One minor observation (not blocking): queryRDSInstancesInRegions always returns nil as its error even when one or more region workers panic/fail — but this is the existing design (the inner function already logs API errors and breaks rather than propagating them), so the panic path is consistent with the non-panic error path. No change needed here unless the design is revisited separately.

The implementation satisfies the issue #997 acceptance criteria completely. LGTM. 🐇

[approve]

---

🧠 Learnings used

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-05-05T15:44:54.463Z
Learning: In the LeanerCloud/CUDly repository, when a CodeRabbit review flags something as "outside-diff" that was already implemented in an earlier commit of the same PR, it is likely a stale-snapshot artifact where the review diff was generated against a pre-fix view. The inline comment convention `(CR pass on PR #<number> Finding N)` is used by the maintainer (cristim) to mark lines that were changed in direct response to a prior CodeRabbit review pass.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-07T18:28:28.139Z
Learning: In LeanerCloud/CUDly `providers/gcp/recommendations.go`, `collectRegion` fans out to all four GCP service clients (computeengine, cloudsql, memorystore, cloudstorage) concurrently (fixed in PR `#1047` commit 95a390f, H-2 finding). `regionResult` gains `cache` and `storage` slices; merge is updated accordingly. The decision was to wire (not trim) memorystore and cloudstorage because both have complete `GetRecommendations` implementations and their no-op `PurchaseCommitment` paths are orthogonal to surfacing advisory recommendations. Regression tests: `TestRegionResult_HasCacheAndStorageFields` and `TestShouldIncludeService_Cache_Storage`.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-07T19:32:26.182Z
Learning: In LeanerCloud/CUDly `providers/aws/services/rds/client.go`, `findOfferingID` validates AZConfig with a switch that has a `default: error` case covering non-empty invalid values (e.g. "typo-az"), not just an empty-string check. A bad non-empty AZConfig previously fell through to `multiAZ==false` in `paginateRDSOfferings` (same mis-buy as the old default). Fixed in PR `#1085` commit 681fd9e81. Regression test added.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-07T19:32:26.182Z
Learning: In LeanerCloud/CUDly `providers/aws/recommendations/parser_services.go`, `parseRDSDetails` uses an exhaustive switch on `DeploymentOption`: "Multi-AZ" maps to "multi-az", "Single-AZ" maps to "single-az", nil leaves AZConfig unset, and any other non-nil value returns an error. Silent defaulting to "single-az" for unknown values was removed in PR `#1085` commit 681fd9e81. Regression test added.

[cristim]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.