ISSUE-039: reload insurance price cache outside the completed TransactionScope

[meketreve]

Adds ISSUE-039 and fixes it: InsurancePriceRefreshService.Refresh() never reloads the insurance price cache, because it calls InsuranceHelper.LoadInsurancePrices() while a completed TransactionScope is still the ambient transaction.

This defect sat behind ISSUE-036 and only became visible once that issue's migration was applied.

What happens

Refresh() at develop (f9ddac2):

using var scope = Db.CreateTransaction();
_ = Db.Query().CommandText("exec usp_RecalculateInsurancePrices").Timeout(120).ExecuteNonQuery();
scope.Complete();
InsuranceHelper.LoadInsurancePrices();

A using declaration disposes at the end of the method, not at Complete(). Complete() only casts the commit vote — the scope remains Transaction.Current until Dispose(). LoadInsurancePrices() therefore runs inside a scope that is already complete, and DbQuery.ExecuteHelper calls connection.Open() (src/Perpetuum/Data/DbQuery.cs:55), which reads Transaction.Current for transacted connection pooling:

System.InvalidOperationException: The current TransactionScope is already complete.
   at System.Transactions.Transaction.get_Current()
   at Microsoft.Data.ProviderBase.DbConnectionPool.GetFromTransactedPool(Transaction& transaction)
   at Microsoft.Data.SqlClient.SqlConnection.Open(SqlConnectionOverrides overrides)
   at Perpetuum.Data.DbQuery.ExecuteHelper[T](Func`2 execute) in DbQuery.cs:line 55
   at Perpetuum.Services.Insurance.InsuranceHelper.LoadInsurancePrices() in InsuranceHelper.cs:line 447
   at Perpetuum.Services.Insurance.InsurancePriceRefreshService.Refresh() in InsurancePriceRefreshService.cs:line 52

The exception reaches the catch in RefreshAsync, so the startup run and every daily run is counted as a failure and logs refresh failed (N consecutive failure(s)). The MERGE itself is not lost — Complete() has already been called, so Dispose() still commits — but the cache reload never happens.

Why it matters

_insurancePrices (src/Perpetuum/Services/Insurance/InsuranceHelper.cs:401) is a static cache populated lazily: on a miss, GetInsurancePrice reads dbo.insuranceprices once and keeps that value for the definition for the rest of the process lifetime. LoadInsurancePrices() is the only path that refreshes an already-cached definition during normal operation — the sole other caller is the ProductionSetInsurance request handler.

So the daily recalculation updates the table while the running server keeps quoting what it cached earlier, and only a restart clears it. That is the same player-visible symptom ISSUE-036 describes, which means applying the ISSUE-036 migration alone does not restore correct prices on a long-running server.

Relationship to ISSUE-036

Until usp_RecalculateInsurancePrices was fixed, the ExecuteNonQuery on the previous line always threw error 217, so execution never reached LoadInsurancePrices(). Applying docs/db_structure/migrations/ISSUE-036-fix-insurance-proc-self-dependency.sql to a local P36.8 database removed the nesting level exceeded exception from the startup log and exposed this one in its place, at the next line of the same method.

Suggestion for ISSUE-036's production verification list: after applying the migration, also confirm the log carries InsurancePriceRefreshService: prices recalculated and cache reloaded. rather than another refresh failed line. Without that step ISSUE-036 can look resolved at the database level while the server still serves stale prices.

The change

Replace the using declaration with a using block that closes immediately after scope.Complete(), so the scope is disposed and Transaction.Current is null again before LoadInsurancePrices() runs.

This is the using (var scope = Db.CreateTransaction()) form already used throughout Perpetuum.RequestHandlers (~25 call sites), so it introduces no new pattern. No logic, no SQL and no transaction boundary changes: the EXEC remains the only statement inside the transaction, which is what the original code already intended.

ISSUE-039 is filed as IN_PROGRESS rather than DONE, matching how ISSUE-036 is tracked: production still has the ISSUE-036 migration pending, and until it is applied there the SQL error masks this code path.

Validation

Local P36.8 database, develop at f9ddac2, full server start and graceful shutdown for each run:

Log	error 217	InvalidOperationException	refresh failed	prices recalculated and cache reloaded
P36.8, ISSUE-036 migration not applied	1	0	1	0
ISSUE-036 migration applied, no code fix	0	1	1	0
ISSUE-036 migration applied, with this fix	0	0	0	1

The run with the fix reached [Online] in 41 s, spawned 3042 flocks — the same count as the run before it — and shut down gracefully to [Off]. Its startup log contains no exception of any kind. Database side after the run: sys.sql_expression_dependencies for the procedure returns exactly the 4 legitimate dependencies, and dbo.insuranceprices holds 67 rows with non-zero fee and payout.

Build: dotnet build PerpetuumServer2.sln -c Release -p:Platform=x64 — 0 errors. The 30 warnings are pre-existing Perpetuum.AdminTool (MVVMTK0034) and WiX warnings; none reference the changed file.

Regression surface

Refresh() is the only method touched. The transaction now disposes one statement earlier than before, which is the intended boundary — LoadInsurancePrices() is a read that was never meant to participate in the recalculation transaction, and it does not write. Threading is unchanged: Refresh() still runs on the Task.Run started by RefreshAsync, guarded by the same _refreshing flag.

🤖 Generated with Claude Code