Skip to content

chore: add tsg link in error messages #43597

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

Merged
Jorge Rangel (jorgerangel-msft) merged 4 commits into from
May 10, 2024
Merged

chore: add tsg link in error messages #43597

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
84e316b
chore: add tsg link in error messages
jorgerangel-msft Apr 11, 2024
a33e057
pr feedback
jorgerangel-msft May 10, 2024
ce38076
Merge branch 'main' of https://github.com/Azure/azure-sdk-for-net int…
jorgerangel-msft May 10, 2024
7f5268b
add missing error parser
jorgerangel-msft May 10, 2024
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
50 changes: 46 additions & 4 deletions sdk/appconfiguration/Azure.Data.AppConfiguration/src/ConfigurationClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,9 @@ public virtual async Task<Response<ConfigurationSetting>> AddConfigurationSettin
case 201:
return await CreateResponseAsync(response, cancellationToken).ConfigureAwait(false);
case 412:
Comment thread
jorgerangel-msft marked this conversation as resolved.
Outdated
case 401:
case 403:
case 429:
throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser());
default:
throw new RequestFailedException(response);
Expand Down Expand Up @@ -240,6 +243,9 @@ public virtual Response<ConfigurationSetting> AddConfigurationSetting(Configurat
case 201:
return CreateResponse(response);
case 412:
case 401:
case 403:
case 429:
throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser());
default:
throw new RequestFailedException(response);
Expand Down Expand Up @@ -310,6 +316,9 @@ public virtual async Task<Response<ConfigurationSetting>> SetConfigurationSettin
{
200 => await CreateResponseAsync(response, cancellationToken).ConfigureAwait(false),
409 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),
401 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),
403 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),
429 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),

// Throws on 412 if resource was modified.
_ => throw new RequestFailedException(response),
Expand Down Expand Up @@ -353,6 +362,9 @@ public virtual Response<ConfigurationSetting> SetConfigurationSetting(Configurat
{
200 => CreateResponse(response),
409 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),
401 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),
403 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),
429 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),

// Throws on 412 if resource was modified.
_ => throw new RequestFailedException(response),
Expand Down Expand Up @@ -442,6 +454,9 @@ private async Task<Response> DeleteConfigurationSettingAsync(string key, string
200 => response,
204 => response,
409 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),
401 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),
403 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),
429 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),

// Throws on 412 if resource was modified.
_ => throw new RequestFailedException(response)
Expand Down Expand Up @@ -471,6 +486,9 @@ private Response DeleteConfigurationSetting(string key, string label, MatchCondi
200 => response,
204 => response,
409 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),
401 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),
403 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),
429 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),

// Throws on 412 if resource was modified.
_ => throw new RequestFailedException(response)
Expand Down Expand Up @@ -596,6 +614,9 @@ internal virtual async Task<Response<ConfigurationSetting>> GetConfigurationSett
{
200 => await CreateResponseAsync(response, cancellationToken).ConfigureAwait(false),
304 => CreateResourceModifiedResponse(response),
401 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),
403 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),
429 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),
_ => throw new RequestFailedException(response),
};
}
Expand Down Expand Up @@ -633,6 +654,9 @@ internal virtual Response<ConfigurationSetting> GetConfigurationSetting(string k
{
200 => CreateResponse(response),
304 => CreateResourceModifiedResponse(response),
401 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),
403 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),
429 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),
_ => throw new RequestFailedException(response),
};
}
Expand Down Expand Up @@ -1386,6 +1410,9 @@ private async ValueTask<Response<ConfigurationSetting>> SetReadOnlyAsync(string
200 => async
? await CreateResponseAsync(response, cancellationToken).ConfigureAwait(false)
: CreateResponse(response),
401 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),
403 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),
429 => throw new RequestFailedException(response, null, new ConfigurationRequestFailedDetailsParser()),
_ => throw new RequestFailedException(response)
};
}
Expand Down Expand Up @@ -1469,16 +1496,31 @@ private static RequestContext CreateRequestContext(ErrorOptions errorOptions, Ca

private class ConfigurationRequestFailedDetailsParser : RequestFailedDetailsParser
{
private const string TroubleshootingText =
Comment thread
jorgerangel-msft marked this conversation as resolved.
Outdated
"For more information about this error, please see the troubleshooting guide at https://aka.ms/azsdk/net/appconfiguration/troubleshoot";
private const string GeneralTsgSectionText = $"{TroubleshootingText}#general-troubleshooting";
private const string LimitIssuesTroubleshootingText = $"{TroubleshootingText}#limit-issues";
private const string AuthenticationTroubleshootingText = $"{TroubleshootingText}#troubleshooting-authentication-issues";
private readonly Dictionary<int, string> _statusCodeToErrorMessage = new()
{
{ 401, AuthenticationTroubleshootingText },
{ 403, AuthenticationTroubleshootingText },
{ 409, "The setting is read only" },
{ 412, "Setting was already present." },
{ 429, LimitIssuesTroubleshootingText },
};
public override bool TryParse(Response response, out ResponseError error, out IDictionary<string, string> data)
{
string errorMessage = _statusCodeToErrorMessage.TryGetValue(response.Status, out string err) ? err : GeneralTsgSectionText;

switch (response.Status)
{
case 409:
error = new ResponseError(null, "The setting is read only");
data = null;
return true;
case 412:
error = new ResponseError(null, "Setting was already present.");
case 401:
case 403:
case 429:
error = new ResponseError(null, errorMessage);
data = null;
return true;
default:
Expand Down
3 changes: 3 additions & 0 deletions sdk/appconfiguration/Azure.Data.AppConfiguration/tests/ConfigurationLiveTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ public async Task DeleteSetting()
[RecordedTest]
public async Task DeleteSettingWithLabel()
{
var troubleshootingLink = "https://aka.ms/azsdk/net/appconfiguration/troubleshoot";
ConfigurationClient service = GetClient();
ConfigurationSetting testSetting = CreateSetting();

Expand All @@ -190,6 +191,8 @@ public async Task DeleteSettingWithLabel()
});

Assert.AreEqual(404, e.Status);
Assert.IsNotEmpty(e.Message);
Assert.IsFalse(e.Message.Contains(troubleshootingLink));
Comment thread
jorgerangel-msft marked this conversation as resolved.
Outdated
}
finally
{
Expand Down
29 changes: 29 additions & 0 deletions sdk/appconfiguration/Azure.Data.AppConfiguration/tests/ConfigurationMockTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ public class ConfigurationMockTests : ClientTestBase
private static readonly string s_credential = "b1d9b31";
private static readonly string s_secret = "aabbccdd";
private static readonly string s_connectionString = $"Endpoint={s_endpoint};Id={s_credential};Secret={s_secret}";
private static readonly string s_troubleshootingLink = "https://aka.ms/azsdk/net/appconfiguration/troubleshoot";
private static readonly string s_version = new ConfigurationClientOptions().Version;

private static readonly ConfigurationSetting s_testSetting = new ConfigurationSetting("test_key", "test_value")
Expand Down Expand Up @@ -108,6 +109,34 @@ public void GetNotFound()
Assert.AreEqual(404, exception.Status);
}

// This test validates that the client throws an exception with the expected error message when it receives a
// non-success status code from the service.
[TestCase((int)HttpStatusCode.Unauthorized, true)]
[TestCase(403, true)]
[TestCase((int)HttpStatusCode.NotFound, false)]
public void GetUnsucessfulResponse(int statusCode, bool containsTsg)
{
var response = new MockResponse(statusCode);
var mockTransport = new MockTransport(response);
ConfigurationClient service = CreateTestService(mockTransport);

RequestFailedException exception = Assert.ThrowsAsync<RequestFailedException>(async () =>
{
await service.GetConfigurationSettingAsync(key: s_testSetting.Key);
});

Assert.AreEqual(statusCode, exception.Status);

if (containsTsg)
{
Assert.True(exception?.Message.Contains(s_troubleshootingLink));
}
else
{
Assert.False(exception?.Message.Contains(s_troubleshootingLink));
}
}

[Test]
public async Task GetIfChangedModified()
{
Expand Down