Skip to content

Commit b48965c

Browse files
StephenMolloyHongGit
StephenMolloy
authored and
HongGit
committed
Exception verbosity (#11)
* Remove extraneous exception wrapping and messaging. * Remove extraneous exception wrapping and messaging. * Catching all exceptions is bad.
1 parent 0d339e0 commit b48965c

File tree

5 files changed

+56
-67
lines changed

5 files changed

+56
-67
lines changed

‎src/Azure/AzureKeyVaultConfigBuilder.cs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
using System.Threading.Tasks;
1010

1111
using Microsoft.Azure.KeyVault;
12+
using Microsoft.Azure.KeyVault.Models;
1213
using Microsoft.Azure.Services.AppAuthentication;
1314
using Microsoft.IdentityModel.Clients.ActiveDirectory;
1415

@@ -47,7 +48,7 @@ public override void Initialize(string name, NameValueCollection config)
4748
if (String.IsNullOrWhiteSpace(_uri))
4849
{
4950
if (String.IsNullOrWhiteSpace(_vaultName))
50-
throw new ArgumentException($"AzureKeyVaultConfigBuilder {name}: Vault must be specified by name or URI using the '{vaultNameTag}' or '{uriTag}' attribute.");
51+
throw new ArgumentException($"Vault must be specified by name or URI using the '{vaultNameTag}' or '{uriTag}' attribute.");
5152
else
5253
_uri = $"https://{_vaultName}.vault.azure.net";
5354
}
@@ -107,7 +108,15 @@ private async Task<string> GetValueAsync(string key)
107108

108109
var secret = await _kvClient.GetSecretAsync(_uri, key);
109110
return secret?.Value;
110-
} catch { }
111+
} catch (KeyVaultErrorException kve) {
112+
// Simply return null if the secret wasn't found
113+
if (kve.Body.Error.Code == "SecretNotFound")
114+
return null;
115+
116+
// If there was a permission issue or some other error, let the exception bubble
117+
// FYI: kve.Body.Error.Code == "Forbidden" :: No Rights, or secret is disabled.
118+
throw;
119+
}
111120
}
112121

113122
return null;

‎src/Base/KeyValueConfigBuilder.cs

Lines changed: 40 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -30,87 +30,67 @@ public abstract class KeyValueConfigBuilder : ConfigurationBuilder
3030

3131
public override void Initialize(string name, NameValueCollection config)
3232
{
33-
try
34-
{
35-
base.Initialize(name, config);
36-
37-
// Override default config
38-
if (config != null)
39-
{
40-
KeyPrefix = config[prefixTag] ?? "";
41-
TokenPattern = config[tokenPatternTag] ?? TokenPattern;
33+
base.Initialize(name, config);
4234

43-
if (config[stripPrefixTag] != null) {
44-
// We want an exception here if 'stripPrefix' is specified but unrecognized.
45-
_stripPrefix = Boolean.Parse(config[stripPrefixTag]);
46-
}
35+
// Override default config
36+
if (config != null)
37+
{
38+
KeyPrefix = config[prefixTag] ?? "";
39+
TokenPattern = config[tokenPatternTag] ?? TokenPattern;
4740

48-
if (config[modeTag] != null) {
49-
// We want an exception here if 'mode' is specified but unrecognized.
50-
Mode = (KeyValueMode)Enum.Parse(typeof(KeyValueMode), config[modeTag], true);
51-
}
41+
if (config[stripPrefixTag] != null) {
42+
// We want an exception here if 'stripPrefix' is specified but unrecognized.
43+
_stripPrefix = Boolean.Parse(config[stripPrefixTag]);
5244
}
5345

54-
_cachedValues = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
55-
}
56-
catch (Exception e)
57-
{
58-
throw new ConfigurationErrorsException($"Error while initializing Configuration Builder '{name}'. ({e.Message})", e);
46+
if (config[modeTag] != null) {
47+
// We want an exception here if 'mode' is specified but unrecognized.
48+
Mode = (KeyValueMode)Enum.Parse(typeof(KeyValueMode), config[modeTag], true);
49+
}
5950
}
51+
52+
_cachedValues = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
6053
}
6154

6255
public override XmlNode ProcessRawXml(XmlNode rawXml)
6356
{
64-
try
65-
{
66-
if (Mode == KeyValueMode.Expand)
67-
return ExpandTokens(rawXml);
57+
if (Mode == KeyValueMode.Expand)
58+
return ExpandTokens(rawXml);
6859

69-
return rawXml;
70-
}
71-
catch (Exception e)
72-
{
73-
throw new ConfigurationErrorsException($"Error while processing xml in Configuration Builder '{Name}'. ({e.Message})", e);
74-
}
60+
return rawXml;
7561
}
7662

7763
public override ConfigurationSection ProcessConfigurationSection(ConfigurationSection configSection)
7864
{
79-
try {
80-
// Expand mode works on the raw string input
81-
if (Mode == KeyValueMode.Expand)
82-
return configSection;
83-
84-
// In Greedy mode, we need to know all the key/value pairs from this config source. So we
85-
// can't 'cache' them as we go along. Slurp them all up now. But only once. ;)
86-
if ((Mode == KeyValueMode.Greedy) && (!_greedyInited))
65+
// Expand mode works on the raw string input
66+
if (Mode == KeyValueMode.Expand)
67+
return configSection;
68+
69+
// In Greedy mode, we need to know all the key/value pairs from this config source. So we
70+
// can't 'cache' them as we go along. Slurp them all up now. But only once. ;)
71+
if ((Mode == KeyValueMode.Greedy) && (!_greedyInited))
72+
{
73+
lock (_cachedValues)
8774
{
88-
lock (_cachedValues)
75+
if (!_greedyInited)
8976
{
90-
if (!_greedyInited)
77+
foreach (KeyValuePair<string, string> kvp in GetAllValuesInternal(KeyPrefix))
9178
{
92-
foreach (KeyValuePair<string, string> kvp in GetAllValuesInternal(KeyPrefix))
93-
{
94-
_cachedValues.Add(kvp);
95-
}
96-
_greedyInited = true;
79+
_cachedValues.Add(kvp);
9780
}
81+
_greedyInited = true;
9882
}
9983
}
84+
}
10085

101-
if (configSection is AppSettingsSection) {
102-
return ProcessAppSettings((AppSettingsSection)configSection);
103-
}
104-
else if (configSection is ConnectionStringsSection) {
105-
return ProcessConnectionStrings((ConnectionStringsSection)configSection);
106-
}
107-
108-
return configSection;
86+
if (configSection is AppSettingsSection) {
87+
return ProcessAppSettings((AppSettingsSection)configSection);
10988
}
110-
catch (Exception e)
111-
{
112-
throw new ConfigurationErrorsException($"Error while processing configSection in Configuration Builder '{Name}'. ({e.Message})", e);
89+
else if (configSection is ConnectionStringsSection) {
90+
return ProcessConnectionStrings((ConnectionStringsSection)configSection);
11391
}
92+
93+
return configSection;
11494
}
11595

11696
private XmlNode ExpandTokens(XmlNode rawXml)
@@ -244,7 +224,7 @@ private string GetValueInternal(string key)
244224
}
245225
catch (Exception e)
246226
{
247-
throw new ConfigurationErrorsException($"Error in Configuration Builder '{Name}'::GetValue({key})", e);
227+
throw new Exception($"Error in Configuration Builder '{Name}'::GetValue({key})", e);
248228
}
249229
}
250230

@@ -256,7 +236,7 @@ private ICollection<KeyValuePair<string, string>> GetAllValuesInternal(string pr
256236
}
257237
catch (Exception e)
258238
{
259-
throw new ConfigurationErrorsException($"Error in Configuration Builder '{Name}'::GetAllValues({prefix})", e);
239+
throw new Exception($"Error in Configuration Builder '{Name}'::GetAllValues({prefix})", e);
260240
}
261241
}
262242
}

‎src/Json/SimpleJsonConfigBuilder.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public override void Initialize(string name, NameValueCollection config)
4444
string jsonFile = config?[jsonFileTag];
4545
if (String.IsNullOrWhiteSpace(jsonFile))
4646
{
47-
throw new ArgumentException($"SimpleJsonConfigBuilder '{name}': Json file must be specified with the '{jsonFileTag}' attribute.");
47+
throw new ArgumentException($"Json file must be specified with the '{jsonFileTag}' attribute.");
4848
}
4949
JsonFile = Utils.MapPath(jsonFile);
5050
if (!File.Exists(JsonFile))
@@ -56,7 +56,7 @@ public override void Initialize(string name, NameValueCollection config)
5656
return;
5757
}
5858

59-
throw new ArgumentException($"SimpleJsonConfigBuilder '{name}': Json file does not exist.");
59+
throw new ArgumentException($"Json file does not exist.");
6060
}
6161

6262
// JsonMode

‎src/UserSecrets/UserSecretsConfigBuilder.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public override void Initialize(string name, NameValueCollection config)
4545
}
4646
else if (!Optional)
4747
{
48-
throw new ArgumentException($"UserSecretsConfigBuilder '{name}': Secrets file does not exist.");
48+
throw new ArgumentException($"Secrets file does not exist.");
4949
}
5050
else
5151
{
@@ -89,7 +89,7 @@ private string GetSecretsFileFromId(string secretsId)
8989
int badCharIndex = secretsId.IndexOfAny(Path.GetInvalidFileNameChars());
9090
if (badCharIndex != -1)
9191
{
92-
throw new InvalidOperationException($"UserSecretsConfigBuilder '{Name}': Invalid character '{secretsId[badCharIndex]}' in '{userSecretsIdTag}'.");
92+
throw new InvalidOperationException($"Invalid character '{secretsId[badCharIndex]}' in '{userSecretsIdTag}'.");
9393
}
9494

9595
// Try Windows-style first

‎test/Microsoft.Configuration.ConfigurationBuilders.Test/Test/BaseTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public void BaseParameters_Mode()
4141

4242
// Invalid
4343
builder = new FakeConfigBuilder();
44-
Assert.ThrowsException<ConfigurationErrorsException>(() => {
44+
Assert.ThrowsException<ArgumentException>(() => {
4545
builder.Initialize("test", new System.Collections.Specialized.NameValueCollection() { { "mode", "InvalidModeDoesNotExist" } });
4646
});
4747

0 commit comments

Comments
 (0)