Skip to content

Commit f2b4cbd

Browse files
aaronburtleCopilotsouvikghosh04
authored
Fix dab validate error messaging when logger is not available (#3311)
## Why make this change? Closes #3268 ## What is this change? When `dab validate` or `dab start` encounters a config parsing error (e.g. missing entities/autoentities), the CLI previously dumped the full exception message and stack trace to stderr. This made the output noisy and unhelpful. After this change, only a clean, descriptive validation message is shown. The key design changes: - `TryParseConfig` is now a pure function that returns an `out string? parseError` message on failure instead of writing to `Console.Error` or `ILogger` internally. Error reporting is the caller's responsibility. - `FileSystemRuntimeConfigLoader.TryLoadConfig` writes the `parseError` to `Console.Error` (because config is parsed before the DI container and logger are available, so the log buffer would never be flushed on a parse failure) and sets an instance-scoped `IsParseErrorEmitted` flag so CLI callers (`ConfigGenerator`) can avoid logging duplicate messages. - `ConfigGenerator.IsConfigValid` now has an explicit early-return path when config parsing fails (via `runtimeConfigProvider.TryGetConfig`), using `IsParseErrorEmitted` to suppress duplicate output. - `ValidateOptions.Handler` uses `LogError("Config is invalid.")`. - Comments referencing which method emits the error to `Console.Error` corrected to `TryLoadConfig` (not `TryParseConfig`). ## How was this tested? * `ValidateConfigTests.cs` — Added `TestValidateConfigWithNoEntitiesProducesCleanError` (new test verifying clean error message, no stack traces). * `EnvironmentTests.cs` — Updated `FailureToStartEngineWhenEnvVarNamedWrong` to match the new single-line clean stderr format. * `EndToEndTests.cs` — Simplified assertions in `TestExitOfRuntimeEngineWithInvalidConfig` (this test is Ignored but updated for consistency). * `RuntimeConfigLoaderTests.cs` — Updated `FailLoadMultiDataSourceConfigDuplicateEntities` to assert `loader.IsParseErrorEmitted` is true. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Souvik Ghosh <souvikofficial04@gmail.com>
1 parent 57ce9cf commit f2b4cbd

11 files changed

Lines changed: 103 additions & 47 deletions

src/Cli.Tests/EndToEndTests.cs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1095,10 +1095,6 @@ public async Task TestExitOfRuntimeEngineWithInvalidConfig(
10951095
Assert.IsNotNull(output);
10961096
StringAssert.Contains(output, $"Deserialization of the configuration file failed.", StringComparison.Ordinal);
10971097

1098-
output = await process.StandardOutput.ReadLineAsync();
1099-
Assert.IsNotNull(output);
1100-
StringAssert.Contains(output, $"Error: Failed to parse the config file: {TEST_RUNTIME_CONFIG_FILE}.", StringComparison.Ordinal);
1101-
11021098
output = await process.StandardOutput.ReadLineAsync();
11031099
Assert.IsNotNull(output);
11041100
StringAssert.Contains(output, $"Failed to start the engine.", StringComparison.Ordinal);

src/Cli.Tests/EnvironmentTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,8 @@ public async Task FailureToStartEngineWhenEnvVarNamedWrong()
163163
);
164164

165165
string? output = await process.StandardError.ReadLineAsync();
166-
Assert.AreEqual("Deserialization of the configuration file failed during a post-processing step.", output);
167-
output = await process.StandardError.ReadToEndAsync();
166+
Assert.IsNotNull(output);
167+
// Clean error message on stderr with no stack trace.
168168
StringAssert.Contains(output, "A valid Connection String should be provided.", StringComparison.Ordinal);
169169
process.Kill();
170170
}

src/Cli.Tests/ValidateConfigTests.cs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,34 @@ public void TestValidateConfigFailsWithNoEntities()
199199
}
200200
}
201201

202+
/// <summary>
203+
/// Validates that when the config has no entities or autoentities, TryParseConfig
204+
/// sets a clean error message (not a raw exception with stack trace) and
205+
/// IsConfigValid returns false without throwing.
206+
/// Regression test for https://github.com/Azure/data-api-builder/issues/3268
207+
/// </summary>
208+
[TestMethod]
209+
public void TestValidateConfigWithNoEntitiesProducesCleanError()
210+
{
211+
string configWithoutEntities = $"{{{SAMPLE_SCHEMA_DATA_SOURCE},{RUNTIME_SECTION}}}";
212+
213+
// Verify TryParseConfig produces a clean error without stack traces.
214+
bool parsed = RuntimeConfigLoader.TryParseConfig(configWithoutEntities, out _, out string? parseError);
215+
216+
Assert.IsFalse(parsed, "Config with no entities should fail to parse.");
217+
Assert.IsNotNull(parseError, "parseError should be set when config parsing fails.");
218+
StringAssert.Contains(parseError,
219+
"Configuration file should contain either at least the entities or autoentities property",
220+
"Parse error should contain the clean validation message.");
221+
Assert.IsFalse(parseError.Contains("StackTrace"),
222+
"Stack trace should not be present in parse error.");
223+
224+
// Verify IsConfigValid also returns false cleanly (no exception thrown).
225+
((MockFileSystem)_fileSystem!).AddFile(TEST_RUNTIME_CONFIG_FILE, configWithoutEntities);
226+
ValidateOptions validateOptions = new(TEST_RUNTIME_CONFIG_FILE);
227+
Assert.IsFalse(ConfigGenerator.IsConfigValid(validateOptions, _runtimeConfigLoader!, _fileSystem!));
228+
}
229+
202230
/// <summary>
203231
/// This Test is used to verify that the validate command is able to catch when data source field is missing.
204232
/// </summary>

src/Cli/Commands/ValidateOptions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public int Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSy
3838
}
3939
else
4040
{
41-
logger.LogError("Config is invalid. Check above logs for details.");
41+
logger.LogError("Config is invalid.");
4242
}
4343

4444
return isValidConfig ? CliReturnCode.SUCCESS : CliReturnCode.GENERAL_ERROR;

src/Cli/ConfigGenerator.cs

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2564,7 +2564,14 @@ public static bool TryStartEngineWithOptions(StartOptions options, FileSystemRun
25642564
// Replaces all the environment variables while deserializing when starting DAB.
25652565
if (!loader.TryLoadKnownConfig(out RuntimeConfig? deserializedRuntimeConfig, replaceEnvVar: true))
25662566
{
2567-
_logger.LogError("Failed to parse the config file: {runtimeConfigFile}.", runtimeConfigFile);
2567+
// When IsParseErrorEmitted is true, TryLoadConfig already emitted the
2568+
// detailed error to Console.Error. Only log a generic message to avoid
2569+
// duplicate output (stderr + stdout).
2570+
if (!loader.IsParseErrorEmitted)
2571+
{
2572+
_logger.LogError("Failed to parse the config file: {runtimeConfigFile}.", runtimeConfigFile);
2573+
}
2574+
25682575
return false;
25692576
}
25702577
else
@@ -2643,6 +2650,19 @@ public static bool IsConfigValid(ValidateOptions options, FileSystemRuntimeConfi
26432650

26442651
RuntimeConfigProvider runtimeConfigProvider = new(loader);
26452652

2653+
if (!runtimeConfigProvider.TryGetConfig(out RuntimeConfig? _))
2654+
{
2655+
// When IsParseErrorEmitted is true, TryLoadConfig already emitted the
2656+
// detailed error to Console.Error. Only log a generic message to avoid
2657+
// duplicate output (stderr + stdout).
2658+
if (!loader.IsParseErrorEmitted)
2659+
{
2660+
_logger.LogError("Failed to parse the config file.");
2661+
}
2662+
2663+
return false;
2664+
}
2665+
26462666
ILogger<RuntimeConfigValidator> runtimeConfigValidatorLogger = LoggerFactoryForCli.CreateLogger<RuntimeConfigValidator>();
26472667
RuntimeConfigValidator runtimeConfigValidator = new(runtimeConfigProvider, fileSystem, runtimeConfigValidatorLogger, true);
26482668

src/Config/FileSystemRuntimeConfigLoader.cs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,12 @@ public class FileSystemRuntimeConfigLoader : RuntimeConfigLoader, IDisposable
8585
/// </summary>
8686
public string ConfigFilePath { get; internal set; }
8787

88+
/// <summary>
89+
/// Indicates whether the most recent TryLoadConfig call encountered a parse error
90+
/// that was already emitted to Console.Error.
91+
/// </summary>
92+
public bool IsParseErrorEmitted { get; private set; }
93+
8894
public FileSystemRuntimeConfigLoader(
8995
IFileSystem fileSystem,
9096
HotReloadEventHandler<HotReloadEventArgs>? handler = null,
@@ -227,6 +233,7 @@ public bool TryLoadConfig(
227233
bool? isDevMode = null,
228234
DeserializationVariableReplacementSettings? replacementSettings = null)
229235
{
236+
IsParseErrorEmitted = false;
230237
if (_fileSystem.File.Exists(path))
231238
{
232239
SendLogToBufferOrLogger(LogLevel.Information, $"Loading config file from {_fileSystem.Path.GetFullPath(path)}.");
@@ -263,11 +270,12 @@ public bool TryLoadConfig(
263270
// Use default replacement settings if none provided
264271
replacementSettings ??= new DeserializationVariableReplacementSettings();
265272

273+
string? parseError = null;
266274
if (!string.IsNullOrEmpty(json) && TryParseConfig(
267275
json,
268276
out RuntimeConfig,
277+
out parseError,
269278
replacementSettings,
270-
logger: null,
271279
connectionString: _connectionString))
272280
{
273281
if (TrySetupConfigFileWatcher())
@@ -303,6 +311,12 @@ public bool TryLoadConfig(
303311
RuntimeConfig = LastValidRuntimeConfig;
304312
}
305313

314+
if (parseError is not null)
315+
{
316+
Console.Error.WriteLine(parseError);
317+
IsParseErrorEmitted = true;
318+
}
319+
306320
config = null;
307321
return false;
308322
}

src/Config/RuntimeConfigLoader.cs

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
using Azure.DataApiBuilder.Product;
1414
using Azure.DataApiBuilder.Service.Exceptions;
1515
using Microsoft.Data.SqlClient;
16-
using Microsoft.Extensions.Logging;
1716
using Microsoft.Extensions.Primitives;
1817
using Npgsql;
1918
using static Azure.DataApiBuilder.Config.DabConfigEvents;
@@ -179,16 +178,34 @@ protected void SignalConfigChanged(string message = "")
179178
/// </summary>
180179
/// <param name="json">JSON that represents the config file.</param>
181180
/// <param name="config">The parsed config, or null if it parsed unsuccessfully.</param>
181+
/// <param name="parseError">A clean error message when parsing fails, or null on success.</param>
182182
/// <param name="replacementSettings">Settings for variable replacement during deserialization. If null, no variable replacement will be performed.</param>
183-
/// <param name="logger">logger to log messages</param>
184183
/// <param name="connectionString">connectionString to add to config if specified</param>
185184
/// <returns>True if the config was parsed, otherwise false.</returns>
186185
public static bool TryParseConfig(string json,
187186
[NotNullWhen(true)] out RuntimeConfig? config,
188187
DeserializationVariableReplacementSettings? replacementSettings = null,
189-
ILogger? logger = null,
190188
string? connectionString = null)
191189
{
190+
return TryParseConfig(json, out config, out _, replacementSettings, connectionString);
191+
}
192+
193+
/// <summary>
194+
/// Parses a JSON string into a <c>RuntimeConfig</c> object for single database scenario.
195+
/// </summary>
196+
/// <param name="json">JSON that represents the config file.</param>
197+
/// <param name="config">The parsed config, or null if it parsed unsuccessfully.</param>
198+
/// <param name="parseError">A clean error message when parsing fails, or null on success.</param>
199+
/// <param name="replacementSettings">Settings for variable replacement during deserialization. If null, no variable replacement will be performed.</param>
200+
/// <param name="connectionString">connectionString to add to config if specified</param>
201+
/// <returns>True if the config was parsed, otherwise false.</returns>
202+
public static bool TryParseConfig(string json,
203+
[NotNullWhen(true)] out RuntimeConfig? config,
204+
out string? parseError,
205+
DeserializationVariableReplacementSettings? replacementSettings = null,
206+
string? connectionString = null)
207+
{
208+
parseError = null;
192209
// First pass: extract AzureKeyVault options if AKV replacement is requested
193210
if (replacementSettings?.DoReplaceAkvVar is true)
194211
{
@@ -263,18 +280,9 @@ public static bool TryParseConfig(string json,
263280
ex is JsonException ||
264281
ex is DataApiBuilderException)
265282
{
266-
string errorMessage = ex is JsonException ? "Deserialization of the configuration file failed." :
267-
"Deserialization of the configuration file failed during a post-processing step.";
268-
269-
// logger can be null when called from CLI
270-
if (logger is null)
271-
{
272-
Console.Error.WriteLine(errorMessage + $"\n" + $"Message:\n {ex.Message}\n" + $"Stack Trace:\n {ex.StackTrace}");
273-
}
274-
else
275-
{
276-
logger.LogError(exception: ex, message: errorMessage);
277-
}
283+
parseError = ex is DataApiBuilderException
284+
? ex.Message
285+
: $"Deserialization of the configuration file failed. {ex.Message}";
278286

279287
config = null;
280288
return false;

src/Core/Configurations/RuntimeConfigProvider.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,7 @@ public async Task<bool> Initialize(
188188
if (RuntimeConfigLoader.TryParseConfig(
189189
configuration,
190190
out RuntimeConfig? runtimeConfig,
191+
out _,
191192
replacementSettings: null))
192193
{
193194
_configLoader.RuntimeConfig = runtimeConfig;
@@ -269,7 +270,7 @@ public async Task<bool> Initialize(
269270

270271
IsLateConfigured = true;
271272

272-
if (RuntimeConfigLoader.TryParseConfig(jsonConfig, out RuntimeConfig? runtimeConfig, replacementSettings))
273+
if (RuntimeConfigLoader.TryParseConfig(jsonConfig, out RuntimeConfig? runtimeConfig, out _, replacementSettings))
273274
{
274275
_configLoader.RuntimeConfig = runtimeConfig.DataSource.DatabaseType switch
275276
{

src/Service.Tests/Configuration/ConfigurationTests.cs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2521,8 +2521,7 @@ public async Task TestSPRestDefaultsForManuallyConstructedConfigs(
25212521
configJson,
25222522
out RuntimeConfig deserializedConfig,
25232523
replacementSettings: new(),
2524-
logger: null,
2525-
GetConnectionStringFromEnvironmentConfig(environment: TestCategory.MSSQL));
2524+
connectionString: GetConnectionStringFromEnvironmentConfig(environment: TestCategory.MSSQL));
25262525
string configFileName = "custom-config.json";
25272526
File.WriteAllText(configFileName, deserializedConfig.ToJson());
25282527
string[] args = new[]
@@ -2609,8 +2608,7 @@ public async Task SanityTestForRestAndGQLRequestsWithoutMultipleMutationFeatureF
26092608
configJson,
26102609
out RuntimeConfig deserializedConfig,
26112610
replacementSettings: new(),
2612-
logger: null,
2613-
GetConnectionStringFromEnvironmentConfig(environment: TestCategory.MSSQL)));
2611+
connectionString: GetConnectionStringFromEnvironmentConfig(environment: TestCategory.MSSQL)));
26142612
string configFileName = "custom-config.json";
26152613
File.WriteAllText(configFileName, deserializedConfig.ToJson());
26162614
string[] args = new[]
@@ -3640,8 +3638,7 @@ public async Task ValidateStrictModeAsDefaultForRestRequestBody(bool includeExtr
36403638
configJson,
36413639
out RuntimeConfig deserializedConfig,
36423640
replacementSettings: new(),
3643-
logger: null,
3644-
GetConnectionStringFromEnvironmentConfig(environment: TestCategory.MSSQL));
3641+
connectionString: GetConnectionStringFromEnvironmentConfig(environment: TestCategory.MSSQL));
36453642
const string CUSTOM_CONFIG = "custom-config.json";
36463643
File.WriteAllText(CUSTOM_CONFIG, deserializedConfig.ToJson());
36473644
string[] args = new[]

src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,11 @@ public async Task FailLoadMultiDataSourceConfigDuplicateEntities(string configPa
9797
Console.SetError(sw);
9898

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

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

106107
/// <summary>

0 commit comments

Comments
 (0)