Skip to content

Fix PostgreSQL health check connection string parsing #3083

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Copilot wants to merge 2 commits into
base: main
Choose a base branch
from
+19 −5
Open

Fix PostgreSQL health check connection string parsing #3083

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d4424e0
Initial plan
Copilot Jan 27, 2026
66b22b8
Fix PostgreSQL health check connection string handling
Copilot Jan 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/Service/HealthCheck/HealthCheckHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ private async Task UpdateDataSourceHealthCheckResultsAsync(ComprehensiveHealthCh
if (comprehensiveHealthCheckReport.Checks != null && runtimeConfig.DataSource.IsDatasourceHealthEnabled)
{
string query = Utilities.GetDatSourceQuery(runtimeConfig.DataSource.DatabaseType);
(int, string?) response = await ExecuteDatasourceQueryCheckAsync(query, runtimeConfig.DataSource.ConnectionString, Utilities.GetDbProviderFactory(runtimeConfig.DataSource.DatabaseType));
(int, string?) response = await ExecuteDatasourceQueryCheckAsync(query, runtimeConfig.DataSource.ConnectionString, Utilities.GetDbProviderFactory(runtimeConfig.DataSource.DatabaseType), runtimeConfig.DataSource.DatabaseType);
bool isResponseTimeWithinThreshold = response.Item1 >= 0 && response.Item1 < runtimeConfig.DataSource.DatasourceThresholdMs;

// Add DataSource Health Check Results
Expand All @@ -182,14 +182,14 @@ private async Task UpdateDataSourceHealthCheckResultsAsync(ComprehensiveHealthCh
}

// Executes the DB Query and keeps track of the response time and error message.
private async Task<(int, string?)> ExecuteDatasourceQueryCheckAsync(string query, string connectionString, DbProviderFactory dbProviderFactory)
private async Task<(int, string?)> ExecuteDatasourceQueryCheckAsync(string query, string connectionString, DbProviderFactory dbProviderFactory, DatabaseType databaseType)
{
string? errorMessage = null;
if (!string.IsNullOrEmpty(query) && !string.IsNullOrEmpty(connectionString))
{
Stopwatch stopwatch = new();
stopwatch.Start();
errorMessage = await _httpUtility.ExecuteDbQueryAsync(query, connectionString, dbProviderFactory);
errorMessage = await _httpUtility.ExecuteDbQueryAsync(query, connectionString, dbProviderFactory, databaseType);
stopwatch.Stop();
return string.IsNullOrEmpty(errorMessage) ? ((int)stopwatch.ElapsedMilliseconds, errorMessage) : (HealthCheckConstants.ERROR_RESPONSE_TIME_MS, errorMessage);
}
Expand Down
4 changes: 2 additions & 2 deletions src/Service/HealthCheck/HttpUtilities.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public HttpUtilities(
}

// Executes the DB query by establishing a connection to the DB.
public async Task<string?> ExecuteDbQueryAsync(string query, string connectionString, DbProviderFactory providerFactory)
public async Task<string?> ExecuteDbQueryAsync(string query, string connectionString, DbProviderFactory providerFactory, DatabaseType databaseType)
{
string? errorMessage = null;
// Execute the query on DB and return the response time.
Expand All @@ -65,7 +65,7 @@ public HttpUtilities(
{
try
{
connection.ConnectionString = connectionString;
connection.ConnectionString = Utilities.NormalizeConnectionString(connectionString, databaseType);
using (DbCommand command = connection.CreateCommand())
{
command.CommandText = query;
Expand Down
14 changes: 14 additions & 0 deletions src/Service/HealthCheck/Utilities.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,5 +69,19 @@ public static string CreateHttpRestQuery(string entityName, int first)
// "EntityName?$first=4"
return $"/{entityName}?$first={first}";
}

public static string NormalizeConnectionString(string connectionString, DatabaseType dbType)
{
switch (dbType)
{
case DatabaseType.PostgreSQL:
return new NpgsqlConnectionStringBuilder(connectionString).ToString();
case DatabaseType.MSSQL:
case DatabaseType.DWSQL:
return new SqlConnectionStringBuilder(connectionString).ToString();
default:
return connectionString;
}
Comment on lines +75 to +84
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The NormalizeConnectionString method should handle exceptions from the connection string builders. Throughout the codebase (e.g., PostgreSqlMetadataProvider.TryGetSchemaFromConnectionString at src/Core/Services/MetadataProviders/PostgreSqlMetadataProvider.cs:47-58, RuntimeConfigLoader.GetPgSqlConnectionStringWithApplicationName at src/Config/RuntimeConfigLoader.cs:416-427), connection string builder instantiation is wrapped in try-catch blocks to handle potential parsing errors. If a malformed connection string is passed here, the exception will propagate up and may not be handled appropriately in the health check context. Consider wrapping the builder instantiation in a try-catch block and either logging the error and returning the original connection string, or allowing the exception to propagate with proper context.

Suggested change
switch (dbType)
{
case DatabaseType.PostgreSQL:
return new NpgsqlConnectionStringBuilder(connectionString).ToString();
case DatabaseType.MSSQL:
case DatabaseType.DWSQL:
return new SqlConnectionStringBuilder(connectionString).ToString();
default:
return connectionString;
}
try
{
switch (dbType)
{
case DatabaseType.PostgreSQL:
return new NpgsqlConnectionStringBuilder(connectionString).ToString();
case DatabaseType.MSSQL:
case DatabaseType.DWSQL:
return new SqlConnectionStringBuilder(connectionString).ToString();
default:
return connectionString;
}
}
catch (Exception)
{
// If the connection string cannot be parsed by the builder,
// return the original string to avoid failing the health check.
return connectionString;
}

Copilot uses AI. Check for mistakes.
}
}
}
Loading