FreeAndNil commented on code in PR #306:
URL: https://github.com/apache/logging-log4net/pull/306#discussion_r3792704028
##########
src/log4net/Util/SystemInfo.cs:
##########
@@ -680,20 +683,90 @@ public static bool TryParse(string s, out short val)
/// <returns>the value for the key, or <see langword="null"/></returns>
public static string? GetAppSetting(string key)
{
- if (IsAndroid)
- return Environment.GetEnvironmentVariable(key); // Android does not
support config files
+ // Android does not support config files, and neither does a runtime that
has trimmed the
+ // configuration system away.
+ if (IsAndroid || _configurationSystemUnavailable)
+ return Environment.GetEnvironmentVariable(key);
try
{
- return ConfigurationManager.AppSettings[key];
+ return ReadAppSetting(key);
}
catch (Exception e) when (!e.IsFatal())
{
- // If an exception is thrown here then it looks like the config file
does not parse correctly.
+ if (IsMissingConfigurationSystem(e))
+ {
+ // There is no configuration system to read - Native AOT trims
System.Configuration away.
+ // That is a property of the runtime rather than a fault, so it is not
reported as an
+ // error, and the environment stands in for the config file as it does
on Android.
+ _configurationSystemUnavailable = true;
+ LogLog.Debug(_declaringType,
+ "No configuration system on this runtime. Using environment
variables for application settings.", e);
+ return Environment.GetEnvironmentVariable(key);
+ }
+
+ // The config file itself does not parse. Report it and treat the
setting as absent, without
+ // falling back to the environment - a broken config file must not
silently change where
+ // settings come from.
LogLog.Error(_declaringType, "Exception while reading
ConfigurationSettings. Check your .config file is well formed XML.", e);
}
return null;
}
+ /// <summary>
+ /// Determines whether <paramref name="exception"/> means that there is no
configuration system
+ /// on this runtime, as opposed to a configuration file that does not parse.
+ /// </summary>
+ /// <param name="exception">the exception thrown while reading an
application setting</param>
+ /// <returns><see langword="true"/> if the configuration system itself is
unavailable</returns>
+ /// <remarks>
+ /// <para>
+ /// The inner exceptions have to be walked, because Native AOT surfaces this
as a
+ /// <see cref="ConfigurationErrorsException"/> - the very type a malformed
file produces. What
+ /// distinguishes it is further down the chain: a <see
cref="MissingMethodException"/> for
+ /// <c>ClientConfigurationHost</c>, whose constructor the trimmer removed.
+ /// </para>
+ /// <para>
+ /// An unrecognized failure is treated as a configuration file problem,
which is the safer way
+ /// round: it is reported rather than silently swallowed.
+ /// </para>
+ /// </remarks>
+ private static bool IsMissingConfigurationSystem(Exception? exception)
+ {
+ for (; exception is not null; exception = exception.InnerException)
+ {
+ if (exception is MissingMethodException or TypeLoadException or
FileNotFoundException
+ or PlatformNotSupportedException or NotSupportedException)
+ {
+ return true;
+ }
+ }
+ return false;
+ }
Review Comment:
@gdziadkiewicz I adjusted the catch logic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
