Add XACT_STATE check after third-party proc calls (#695)#759
Merged
erikdarlingdata merged 4 commits intodevfrom Mar 30, 2026
Merged
Add XACT_STATE check after third-party proc calls (#695)#759erikdarlingdata merged 4 commits intodevfrom
erikdarlingdata merged 4 commits intodevfrom
Conversation
…755) Phase 1 extraction of shared installation logic into Installer.Core class library. InstallerGui and Installer.Tests now consume the shared library instead of duplicating code. CLI Installer refactor is next. New files: - Installer.Core/InstallationService.cs — all static install/upgrade methods - Installer.Core/DependencyInstaller.cs — community dependency downloads - Installer.Core/ScriptProvider.cs — filesystem + embedded resource abstraction - Installer.Core/Patterns.cs — shared regex patterns ([GeneratedRegex]) - Installer.Core/Models/ — InstallationProgress, ServerInfo, InstallationResult, UpgradeInfo, InstallationResultCode (enum mapping CLI exit codes 0-8) Key changes: - SQL scripts embedded as assembly resources for future Dashboard integration - ScriptProvider.FromDirectory() for CLI/GUI, FromEmbeddedResources() for Dashboard - AutoDiscover() searches filesystem then falls back to embedded - Comprehensive [DEBUG] logging throughout all methods for GUI diagnostics - upgrade.txt missing warning (was silently skipped, now logged) - GenerateSummaryReport gains optional outputDirectory parameter Retargeted: - InstallerGui references Installer.Core, old InstallationService.cs deleted - Installer.Tests targets net8.0 (was net8.0-windows), no WPF dependency - Tests use ScriptProvider.FromDirectory() instead of raw file paths Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Program.cs rewritten from 2,122 lines to 1,172 lines (45% reduction). All duplicated logic removed — CLI now delegates to Installer.Core for: - Connection string building (InstallationService.BuildConnectionString) - Connection testing (InstallationService.TestConnectionAsync) - Script discovery (ScriptProvider.FromDirectory) - SQL file execution (InstallationService.ExecuteInstallationAsync) - Upgrade detection and execution (ExecuteAllUpgradesAsync) - Uninstall (InstallationService.ExecuteUninstallAsync) - Version detection (InstallationService.GetInstalledVersionAsync) - Community dependencies (DependencyInstaller) - Validation (InstallationService.RunValidationAsync) - Installation history (InstallationService.LogInstallationHistoryAsync) - Summary reports (InstallationService.GenerateSummaryReport) What stays in Program.cs (console-specific): - Argument parsing, interactive prompts, retry loop - Console output helpers (WriteSuccess/WriteError/WriteWarning) - ReadPassword, WaitForExit, CheckForInstallerUpdateAsync - Error log generation (WriteErrorLog) Fixes during review: - Added missing exit code 8 (UpgradesFailed) to --help text - Fixed encryption default label in error message (was "optional", is "mandatory") Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ollection_table (#756) When calculate_deltas was called inside a collector's transaction and failed, the CATCH block tried to INSERT into collection_log while the transaction was doomed (XACT_STATE = -1), swallowing the real error with "The current transaction cannot be committed." Same pattern in ensure_collection_table where INSERT happened before ROLLBACK. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…695) sp_BlitzLock and sp_HumanEventsBlockViewer can fail internally and doom the caller's transaction. Without checking XACT_STATE after the call, the next write attempt produces "cannot be committed" which swallows the real error. Now we detect the doomed state, rollback, and surface a meaningful error message. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
process_deadlock_xml: CheckXACT_STATE()after callingsp_BlitzLock— if the transaction is doomed, rollback and surface a meaningful error instead of "cannot be committed"process_blocked_process_xml: Same fix forsp_HumanEventsBlockViewerRelated to #695 where a user saw the generic "cannot be committed" error from
process_deadlock_xmlwith no way to diagnose the actual sp_BlitzLock failure.Neither
sp_WhoIsActivenorsp_HealthParserneed this fix — they are not called inside a transaction.Test plan
@debug = 1query_snapshots_collectorandcollect_system_health_wrapperdon't use transactions (no fix needed)🤖 Generated with Claude Code