From c87944c884e4609e9099ca5e965e375cd1aef77f Mon Sep 17 00:00:00 2001 From: Haacked Date: Fri, 18 Oct 2013 11:08:56 -0700 Subject: [PATCH] Add default messages for custom exceptions Right now, our exception messages come from the API response in most cases. But if for any reason there isn't one, we supply a default. I realized these messages might make it to people's logs so it might be nice to spend time later making them even more useful. --- Octokit.Tests/Exceptions/ApiExceptionTests.cs | 1 + .../Exceptions/ApiValidationExceptionTests.cs | 13 ++++++++ .../Exceptions/ForbiddenExceptionTests.cs | 9 ++++++ .../LoginAttemptsExceededExceptionTests.cs | 30 +++++++++++++++++++ .../RateLimitExceededExceptionTests.cs | 2 ++ .../TwoFactorChallengeFailedException.cs | 18 +++++++++++ .../TwoFactorRequiredExceptionTests.cs | 30 +++++++++++++++++++ Octokit.Tests/Octokit.Tests.csproj | 3 ++ Octokit.Tests/OctokitRT.Tests.csproj | 3 ++ Octokit/Exceptions/ApiException.cs | 2 +- Octokit/Exceptions/ApiValidationException.cs | 5 ++++ .../LoginAttemptsExceededException.cs | 5 ++++ .../Exceptions/RateLimitExceededException.cs | 7 +++++ .../TwoFactorChallengeFailedException.cs | 5 ++++ .../Exceptions/TwoFactorRequiredException.cs | 5 ++++ 15 files changed, 137 insertions(+), 1 deletion(-) create mode 100644 Octokit.Tests/Exceptions/LoginAttemptsExceededExceptionTests.cs create mode 100644 Octokit.Tests/Exceptions/TwoFactorChallengeFailedException.cs create mode 100644 Octokit.Tests/Exceptions/TwoFactorRequiredExceptionTests.cs diff --git a/Octokit.Tests/Exceptions/ApiExceptionTests.cs b/Octokit.Tests/Exceptions/ApiExceptionTests.cs index ea3e6097..ec4afe61 100644 --- a/Octokit.Tests/Exceptions/ApiExceptionTests.cs +++ b/Octokit.Tests/Exceptions/ApiExceptionTests.cs @@ -34,6 +34,7 @@ namespace Octokit.Tests.Exceptions [InlineData("")] [InlineData(null)] [InlineData("{{{{{")] + [InlineData("

502 Bad Gateway

The server returned an invalid or incomplete response.")] public void CreatesGitHubErrorIfResponseMessageIsNotValidJson(string responseContent) { var response = new ApiResponse diff --git a/Octokit.Tests/Exceptions/ApiValidationExceptionTests.cs b/Octokit.Tests/Exceptions/ApiValidationExceptionTests.cs index f43995d4..a53af06f 100644 --- a/Octokit.Tests/Exceptions/ApiValidationExceptionTests.cs +++ b/Octokit.Tests/Exceptions/ApiValidationExceptionTests.cs @@ -27,6 +27,19 @@ namespace Octokit.Tests.Exceptions Assert.Equal("key is already in use", exception.ApiError.Errors.First().Message); } + [Fact] + public void ProvidesDefaultMessage() + { + var response = new ApiResponse + { + StatusCode = (HttpStatusCode)422 + }; + + var exception = new ApiValidationException(response); + + Assert.Equal("Validation Failed", exception.Message); + } + #if !NETFX_CORE [Fact] public void CanPopulateObjectFromSerializedData() diff --git a/Octokit.Tests/Exceptions/ForbiddenExceptionTests.cs b/Octokit.Tests/Exceptions/ForbiddenExceptionTests.cs index ee694fcd..2f339e70 100644 --- a/Octokit.Tests/Exceptions/ForbiddenExceptionTests.cs +++ b/Octokit.Tests/Exceptions/ForbiddenExceptionTests.cs @@ -18,6 +18,15 @@ namespace Octokit.Tests.Exceptions Assert.Equal("YOU SHALL NOT PASS!", forbiddenException.ApiError.Message); } + + [Fact] + public void HasDefaultMessage() + { + var response = new ApiResponse { StatusCode = HttpStatusCode.Forbidden }; + var forbiddenException = new ForbiddenException(response); + + Assert.Equal("Request Forbidden", forbiddenException.Message); + } } } } diff --git a/Octokit.Tests/Exceptions/LoginAttemptsExceededExceptionTests.cs b/Octokit.Tests/Exceptions/LoginAttemptsExceededExceptionTests.cs new file mode 100644 index 00000000..c2f38249 --- /dev/null +++ b/Octokit.Tests/Exceptions/LoginAttemptsExceededExceptionTests.cs @@ -0,0 +1,30 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Net; +using System.Text; +using System.Threading.Tasks; +using Octokit.Internal; +using Xunit; + +namespace Octokit.Tests.Exceptions +{ + public class LoginAttemptsExceededExceptionTests + { + public class TheConstructor + { + [Fact] + public void SetsDefaultMessage() + { + var response = new ApiResponse + { + StatusCode = HttpStatusCode.Forbidden + }; + + var exception = new LoginAttemptsExceededException(response); + + Assert.Equal("Maximum number of login attempts exceeded", exception.Message); + } + } + } +} diff --git a/Octokit.Tests/Exceptions/RateLimitExceededExceptionTests.cs b/Octokit.Tests/Exceptions/RateLimitExceededExceptionTests.cs index 266e3c78..b62479d5 100644 --- a/Octokit.Tests/Exceptions/RateLimitExceededExceptionTests.cs +++ b/Octokit.Tests/Exceptions/RateLimitExceededExceptionTests.cs @@ -28,6 +28,8 @@ namespace Octokit.Tests.Exceptions "Mon 01 Jul 2013 5:47:53 PM -00:00", "ddd dd MMM yyyy h:mm:ss tt zzz", CultureInfo.InvariantCulture); + + Assert.Equal("API Rate Limit exceeded", exception.Message); Assert.Equal(expectedReset, exception.Reset); } diff --git a/Octokit.Tests/Exceptions/TwoFactorChallengeFailedException.cs b/Octokit.Tests/Exceptions/TwoFactorChallengeFailedException.cs new file mode 100644 index 00000000..7cda26d3 --- /dev/null +++ b/Octokit.Tests/Exceptions/TwoFactorChallengeFailedException.cs @@ -0,0 +1,18 @@ +using Xunit; + +namespace Octokit.Tests.Exceptions +{ + public class TwoFactorChallengeFailedExceptionTests + { + public class TheConstructor + { + [Fact] + public void SetsDefaultMessage() + { + var exception = new TwoFactorChallengeFailedException(); + + Assert.Equal("The two-factor authentication code supplied is not correct", exception.Message); + } + } + } +} diff --git a/Octokit.Tests/Exceptions/TwoFactorRequiredExceptionTests.cs b/Octokit.Tests/Exceptions/TwoFactorRequiredExceptionTests.cs new file mode 100644 index 00000000..605d7153 --- /dev/null +++ b/Octokit.Tests/Exceptions/TwoFactorRequiredExceptionTests.cs @@ -0,0 +1,30 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Net; +using System.Text; +using System.Threading.Tasks; +using Octokit.Internal; +using Xunit; + +namespace Octokit.Tests.Exceptions +{ + public class TwoFactorRequiredExceptionTests + { + public class TheConstructor + { + [Fact] + public void SetsDefaultMessage() + { + var response = new ApiResponse + { + StatusCode = HttpStatusCode.Unauthorized + }; + + var exception = new TwoFactorRequiredException(response, TwoFactorType.Sms); + + Assert.Equal("Two-factor authentication code is required", exception.Message); + } + } + } +} diff --git a/Octokit.Tests/Octokit.Tests.csproj b/Octokit.Tests/Octokit.Tests.csproj index 943fbd73..ee908dd7 100644 --- a/Octokit.Tests/Octokit.Tests.csproj +++ b/Octokit.Tests/Octokit.Tests.csproj @@ -64,8 +64,11 @@ + + + diff --git a/Octokit.Tests/OctokitRT.Tests.csproj b/Octokit.Tests/OctokitRT.Tests.csproj index 24aec30b..f54cdd7d 100644 --- a/Octokit.Tests/OctokitRT.Tests.csproj +++ b/Octokit.Tests/OctokitRT.Tests.csproj @@ -57,7 +57,10 @@ + + + diff --git a/Octokit/Exceptions/ApiException.cs b/Octokit/Exceptions/ApiException.cs index 96ccf9f7..2aa9fe72 100644 --- a/Octokit/Exceptions/ApiException.cs +++ b/Octokit/Exceptions/ApiException.cs @@ -36,7 +36,7 @@ namespace Octokit public override string Message { - get { return ApiErrorMessageSafe; } + get { return ApiErrorMessageSafe ?? "Request Forbidden"; } } public HttpStatusCode StatusCode { get; private set; } diff --git a/Octokit/Exceptions/ApiValidationException.cs b/Octokit/Exceptions/ApiValidationException.cs index 625ef268..91987093 100644 --- a/Octokit/Exceptions/ApiValidationException.cs +++ b/Octokit/Exceptions/ApiValidationException.cs @@ -33,6 +33,11 @@ namespace Octokit "ApiValidationException created with wrong status code"); } + public override string Message + { + get { return ApiErrorMessageSafe ?? "Validation Failed"; } + } + #if !NETFX_CORE protected ApiValidationException(SerializationInfo info, StreamingContext context) : base(info, context) diff --git a/Octokit/Exceptions/LoginAttemptsExceededException.cs b/Octokit/Exceptions/LoginAttemptsExceededException.cs index 57ced2bd..3d76b7dd 100644 --- a/Octokit/Exceptions/LoginAttemptsExceededException.cs +++ b/Octokit/Exceptions/LoginAttemptsExceededException.cs @@ -20,6 +20,11 @@ namespace Octokit { } + public override string Message + { + get { return ApiErrorMessageSafe ?? "Maximum number of login attempts exceeded"; } + } + #if !NETFX_CORE protected LoginAttemptsExceededException(SerializationInfo info, StreamingContext context) : base(info, context) diff --git a/Octokit/Exceptions/RateLimitExceededException.cs b/Octokit/Exceptions/RateLimitExceededException.cs index 4919288d..a9d98327 100644 --- a/Octokit/Exceptions/RateLimitExceededException.cs +++ b/Octokit/Exceptions/RateLimitExceededException.cs @@ -52,6 +52,13 @@ namespace Octokit /// public DateTimeOffset Reset { get; private set; } + // TODO: Might be nice to have this provide a more detailed message such as what the limit is, + // how many are remaining, and when it will reset. I'm too lazy to do it now. + public override string Message + { + get { return ApiErrorMessageSafe ?? "API Rate Limit exceeded"; } + } + #if !NETFX_CORE protected RateLimitExceededException(SerializationInfo info, StreamingContext context) : base(info, context) diff --git a/Octokit/Exceptions/TwoFactorChallengeFailedException.cs b/Octokit/Exceptions/TwoFactorChallengeFailedException.cs index d25f85e3..0afa790e 100644 --- a/Octokit/Exceptions/TwoFactorChallengeFailedException.cs +++ b/Octokit/Exceptions/TwoFactorChallengeFailedException.cs @@ -23,6 +23,11 @@ namespace Octokit { } + public override string Message + { + get { return "The two-factor authentication code supplied is not correct"; } + } + #if !NETFX_CORE protected TwoFactorChallengeFailedException(SerializationInfo info, StreamingContext context) : base(info, context) diff --git a/Octokit/Exceptions/TwoFactorRequiredException.cs b/Octokit/Exceptions/TwoFactorRequiredException.cs index f256f028..3b4e3384 100644 --- a/Octokit/Exceptions/TwoFactorRequiredException.cs +++ b/Octokit/Exceptions/TwoFactorRequiredException.cs @@ -31,6 +31,11 @@ namespace Octokit TwoFactorType = twoFactorType; } + public override string Message + { + get { return ApiErrorMessageSafe ?? "Two-factor authentication code is required"; } + } + #if !NETFX_CORE protected TwoFactorRequiredException(SerializationInfo info, StreamingContext context) : base(info, context)