Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
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
4 changes: 0 additions & 4 deletions src/Cli.Tests/EndToEndTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1095,10 +1095,6 @@ public async Task TestExitOfRuntimeEngineWithInvalidConfig(
Assert.IsNotNull(output);
StringAssert.Contains(output, $"Deserialization of the configuration file failed.", StringComparison.Ordinal);

output = await process.StandardOutput.ReadLineAsync();
Assert.IsNotNull(output);
StringAssert.Contains(output, $"Error: Failed to parse the config file: {TEST_RUNTIME_CONFIG_FILE}.", StringComparison.Ordinal);

output = await process.StandardOutput.ReadLineAsync();
Assert.IsNotNull(output);
StringAssert.Contains(output, $"Failed to start the engine.", StringComparison.Ordinal);
Expand Down
4 changes: 2 additions & 2 deletions src/Cli.Tests/EnvironmentTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,8 @@ public async Task FailureToStartEngineWhenEnvVarNamedWrong()
);

string? output = await process.StandardError.ReadLineAsync();
Assert.AreEqual("Deserialization of the configuration file failed during a post-processing step.", output);
output = await process.StandardError.ReadToEndAsync();
Assert.IsNotNull(output);
// Clean error message on stderr with no stack trace.
StringAssert.Contains(output, "A valid Connection String should be provided.", StringComparison.Ordinal);
process.Kill();
}
Expand Down
28 changes: 28 additions & 0 deletions src/Cli.Tests/ValidateConfigTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,34 @@ public void TestValidateConfigFailsWithNoEntities()
}
}

/// <summary>
/// Validates that when the config has no entities or autoentities, TryParseConfig
/// sets a clean error message (not a raw exception with stack trace) and
/// IsConfigValid returns false without throwing.
/// Regression test for https://github.com/Azure/data-api-builder/issues/3268
/// </summary>
[TestMethod]
public void TestValidateConfigWithNoEntitiesProducesCleanError()
{
string configWithoutEntities = $"{{{SAMPLE_SCHEMA_DATA_SOURCE},{RUNTIME_SECTION}}}";

// Verify TryParseConfig produces a clean error without stack traces.
bool parsed = RuntimeConfigLoader.TryParseConfig(configWithoutEntities, out _, out string? parseError);

Assert.IsFalse(parsed, "Config with no entities should fail to parse.");
Assert.IsNotNull(parseError, "parseError should be set when config parsing fails.");
StringAssert.Contains(parseError,
"Configuration file should contain either at least the entities or autoentities property",
"Parse error should contain the clean validation message.");
Assert.IsFalse(parseError.Contains("StackTrace"),
"Stack trace should not be present in parse error.");

// Verify IsConfigValid also returns false cleanly (no exception thrown).
((MockFileSystem)_fileSystem!).AddFile(TEST_RUNTIME_CONFIG_FILE, configWithoutEntities);
ValidateOptions validateOptions = new(TEST_RUNTIME_CONFIG_FILE);
Assert.IsFalse(ConfigGenerator.IsConfigValid(validateOptions, _runtimeConfigLoader!, _fileSystem!));
}

/// <summary>
/// This Test is used to verify that the validate command is able to catch when data source field is missing.
/// </summary>
Expand Down
2 changes: 1 addition & 1 deletion src/Cli/Commands/ValidateOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public int Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSy
}
else
{
logger.LogError("Config is invalid. Check above logs for details.");
logger.LogError("Config is invalid.");
}

return isValidConfig ? CliReturnCode.SUCCESS : CliReturnCode.GENERAL_ERROR;
Expand Down
22 changes: 21 additions & 1 deletion src/Cli/ConfigGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2564,7 +2564,14 @@ public static bool TryStartEngineWithOptions(StartOptions options, FileSystemRun
// Replaces all the environment variables while deserializing when starting DAB.
if (!loader.TryLoadKnownConfig(out RuntimeConfig? deserializedRuntimeConfig, replaceEnvVar: true))
{
_logger.LogError("Failed to parse the config file: {runtimeConfigFile}.", runtimeConfigFile);
// When IsParseErrorEmitted is true, TryLoadConfig already emitted the
// detailed error to Console.Error. Only log a generic message to avoid
// duplicate output (stderr + stdout).
if (!loader.IsParseErrorEmitted)
{
_logger.LogError("Failed to parse the config file: {runtimeConfigFile}.", runtimeConfigFile);
}

return false;
}
else
Expand Down Expand Up @@ -2643,6 +2650,19 @@ public static bool IsConfigValid(ValidateOptions options, FileSystemRuntimeConfi

RuntimeConfigProvider runtimeConfigProvider = new(loader);

if (!runtimeConfigProvider.TryGetConfig(out RuntimeConfig? _))
{
// When IsParseErrorEmitted is true, TryLoadConfig already emitted the
// detailed error to Console.Error. Only log a generic message to avoid
// duplicate output (stderr + stdout).
if (!loader.IsParseErrorEmitted)
{
_logger.LogError("Failed to parse the config file.");
}

return false;
}

ILogger<RuntimeConfigValidator> runtimeConfigValidatorLogger = LoggerFactoryForCli.CreateLogger<RuntimeConfigValidator>();
RuntimeConfigValidator runtimeConfigValidator = new(runtimeConfigProvider, fileSystem, runtimeConfigValidatorLogger, true);

Expand Down
16 changes: 15 additions & 1 deletion src/Config/FileSystemRuntimeConfigLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,12 @@ public class FileSystemRuntimeConfigLoader : RuntimeConfigLoader, IDisposable
/// </summary>
public string ConfigFilePath { get; internal set; }

/// <summary>
/// Indicates whether the most recent TryLoadConfig call encountered a parse error
/// that was already emitted to Console.Error.
/// </summary>
public bool IsParseErrorEmitted { get; private set; }

public FileSystemRuntimeConfigLoader(
IFileSystem fileSystem,
HotReloadEventHandler<HotReloadEventArgs>? handler = null,
Expand Down Expand Up @@ -227,6 +233,7 @@ public bool TryLoadConfig(
bool? isDevMode = null,
DeserializationVariableReplacementSettings? replacementSettings = null)
{
IsParseErrorEmitted = false;
if (_fileSystem.File.Exists(path))
{
SendLogToBufferOrLogger(LogLevel.Information, $"Loading config file from {_fileSystem.Path.GetFullPath(path)}.");
Expand Down Expand Up @@ -263,11 +270,12 @@ public bool TryLoadConfig(
// Use default replacement settings if none provided
replacementSettings ??= new DeserializationVariableReplacementSettings();

string? parseError = null;
if (!string.IsNullOrEmpty(json) && TryParseConfig(
json,
out RuntimeConfig,
out parseError,
replacementSettings,
logger: null,
connectionString: _connectionString))
{
if (TrySetupConfigFileWatcher())
Expand Down Expand Up @@ -303,6 +311,12 @@ public bool TryLoadConfig(
RuntimeConfig = LastValidRuntimeConfig;
}

if (parseError is not null)
{
Console.Error.WriteLine(parseError);
IsParseErrorEmitted = true;
}

config = null;
return false;
}
Expand Down
38 changes: 23 additions & 15 deletions src/Config/RuntimeConfigLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
using Azure.DataApiBuilder.Product;
using Azure.DataApiBuilder.Service.Exceptions;
using Microsoft.Data.SqlClient;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Primitives;
using Npgsql;
using static Azure.DataApiBuilder.Config.DabConfigEvents;
Expand Down Expand Up @@ -179,16 +178,34 @@ protected void SignalConfigChanged(string message = "")
/// </summary>
/// <param name="json">JSON that represents the config file.</param>
/// <param name="config">The parsed config, or null if it parsed unsuccessfully.</param>
/// <param name="parseError">A clean error message when parsing fails, or null on success.</param>
/// <param name="replacementSettings">Settings for variable replacement during deserialization. If null, no variable replacement will be performed.</param>
/// <param name="logger">logger to log messages</param>
/// <param name="connectionString">connectionString to add to config if specified</param>
/// <returns>True if the config was parsed, otherwise false.</returns>
public static bool TryParseConfig(string json,
[NotNullWhen(true)] out RuntimeConfig? config,
DeserializationVariableReplacementSettings? replacementSettings = null,
ILogger? logger = null,
string? connectionString = null)
{
return TryParseConfig(json, out config, out _, replacementSettings, connectionString);
}

/// <summary>
/// Parses a JSON string into a <c>RuntimeConfig</c> object for single database scenario.
/// </summary>
/// <param name="json">JSON that represents the config file.</param>
/// <param name="config">The parsed config, or null if it parsed unsuccessfully.</param>
/// <param name="parseError">A clean error message when parsing fails, or null on success.</param>
/// <param name="replacementSettings">Settings for variable replacement during deserialization. If null, no variable replacement will be performed.</param>
/// <param name="connectionString">connectionString to add to config if specified</param>
/// <returns>True if the config was parsed, otherwise false.</returns>
public static bool TryParseConfig(string json,
[NotNullWhen(true)] out RuntimeConfig? config,
out string? parseError,
DeserializationVariableReplacementSettings? replacementSettings = null,
string? connectionString = null)
{
parseError = null;
// First pass: extract AzureKeyVault options if AKV replacement is requested
if (replacementSettings?.DoReplaceAkvVar is true)
{
Expand Down Expand Up @@ -263,18 +280,9 @@ public static bool TryParseConfig(string json,
ex is JsonException ||
ex is DataApiBuilderException)
{
string errorMessage = ex is JsonException ? "Deserialization of the configuration file failed." :
"Deserialization of the configuration file failed during a post-processing step.";

// logger can be null when called from CLI
if (logger is null)
{
Console.Error.WriteLine(errorMessage + $"\n" + $"Message:\n {ex.Message}\n" + $"Stack Trace:\n {ex.StackTrace}");
}
else
{
logger.LogError(exception: ex, message: errorMessage);
}
parseError = ex is DataApiBuilderException
? ex.Message
: $"Deserialization of the configuration file failed. {ex.Message}";

config = null;
return false;
Expand Down
3 changes: 2 additions & 1 deletion src/Core/Configurations/RuntimeConfigProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ public async Task<bool> Initialize(
if (RuntimeConfigLoader.TryParseConfig(
configuration,
out RuntimeConfig? runtimeConfig,
out _,
replacementSettings: null))
{
_configLoader.RuntimeConfig = runtimeConfig;
Expand Down Expand Up @@ -269,7 +270,7 @@ public async Task<bool> Initialize(

IsLateConfigured = true;

if (RuntimeConfigLoader.TryParseConfig(jsonConfig, out RuntimeConfig? runtimeConfig, replacementSettings))
if (RuntimeConfigLoader.TryParseConfig(jsonConfig, out RuntimeConfig? runtimeConfig, out _, replacementSettings))
{
_configLoader.RuntimeConfig = runtimeConfig.DataSource.DatabaseType switch
{
Expand Down
9 changes: 3 additions & 6 deletions src/Service.Tests/Configuration/ConfigurationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2521,8 +2521,7 @@ public async Task TestSPRestDefaultsForManuallyConstructedConfigs(
configJson,
out RuntimeConfig deserializedConfig,
replacementSettings: new(),
logger: null,
GetConnectionStringFromEnvironmentConfig(environment: TestCategory.MSSQL));
connectionString: GetConnectionStringFromEnvironmentConfig(environment: TestCategory.MSSQL));
string configFileName = "custom-config.json";
File.WriteAllText(configFileName, deserializedConfig.ToJson());
string[] args = new[]
Expand Down Expand Up @@ -2609,8 +2608,7 @@ public async Task SanityTestForRestAndGQLRequestsWithoutMultipleMutationFeatureF
configJson,
out RuntimeConfig deserializedConfig,
replacementSettings: new(),
logger: null,
GetConnectionStringFromEnvironmentConfig(environment: TestCategory.MSSQL)));
connectionString: GetConnectionStringFromEnvironmentConfig(environment: TestCategory.MSSQL)));
string configFileName = "custom-config.json";
File.WriteAllText(configFileName, deserializedConfig.ToJson());
string[] args = new[]
Expand Down Expand Up @@ -3640,8 +3638,7 @@ public async Task ValidateStrictModeAsDefaultForRestRequestBody(bool includeExtr
configJson,
out RuntimeConfig deserializedConfig,
replacementSettings: new(),
logger: null,
GetConnectionStringFromEnvironmentConfig(environment: TestCategory.MSSQL));
connectionString: GetConnectionStringFromEnvironmentConfig(environment: TestCategory.MSSQL));
const string CUSTOM_CONFIG = "custom-config.json";
File.WriteAllText(CUSTOM_CONFIG, deserializedConfig.ToJson());
string[] args = new[]
Expand Down
7 changes: 4 additions & 3 deletions src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,11 @@ public async Task FailLoadMultiDataSourceConfigDuplicateEntities(string configPa
Console.SetError(sw);

loader.TryLoadConfig("dab-config.json", out RuntimeConfig _);
string error = sw.ToString();

Assert.IsTrue(error.StartsWith("Deserialization of the configuration file failed during a post-processing step."));
Assert.IsTrue(error.Contains("An item with the same key has already been added."));
Assert.IsTrue(loader.IsParseErrorEmitted,
"IsParseErrorEmitted should be true when config parsing fails.");
Assert.IsFalse(string.IsNullOrWhiteSpace(sw.ToString()),
"An error message should have been emitted to Console.Error.");
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@
using Azure.DataApiBuilder.Config.ObjectModel;
using Azure.DataApiBuilder.Service.Exceptions;
using Microsoft.Data.SqlClient;
using Microsoft.Extensions.Logging;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Moq;

namespace Azure.DataApiBuilder.Service.Tests.UnitTests
{
Expand Down Expand Up @@ -325,28 +323,21 @@ public void TestTelemetryApplicationInsightsEnabledShouldError(string configValu
""entities"": { }
}";

// Arrange
Mock<ILogger> mockLogger = new();

// Act
bool isParsed = RuntimeConfigLoader.TryParseConfig(
configJson,
out RuntimeConfig runtimeConfig,
out string parseError,
replacementSettings: new DeserializationVariableReplacementSettings(
azureKeyVaultOptions: null,
doReplaceEnvVar: true,
doReplaceAkvVar: false),
logger: mockLogger.Object);
doReplaceAkvVar: false));

// Assert
Assert.IsFalse(isParsed);
Assert.IsNull(runtimeConfig);

Assert.AreEqual(1, mockLogger.Invocations.Count, "Should raise 1 exception");
Assert.AreEqual(5, mockLogger.Invocations[0].Arguments.Count, "Log should have 4 arguments");
var ConfigException = mockLogger.Invocations[0].Arguments[3] as JsonException;
Assert.IsInstanceOfType(ConfigException, typeof(JsonException), "Should have raised a Json Exception");
Assert.AreEqual(message, ConfigException.Message);
Assert.IsNotNull(parseError, "parseError should be set when parsing fails.");
StringAssert.Contains(parseError, message, "parseError should contain the expected error message.");
}

/// <summary>
Expand Down