diff --git a/Octokit.Tests/Clients/AuthorizationsClientTests.cs b/Octokit.Tests/Clients/AuthorizationsClientTests.cs index fdadb792..4938af30 100644 --- a/Octokit.Tests/Clients/AuthorizationsClientTests.cs +++ b/Octokit.Tests/Clients/AuthorizationsClientTests.cs @@ -1,7 +1,9 @@ using System; using System.Collections.Generic; +using System.Net; using System.Threading.Tasks; using NSubstitute; +using Octokit.Internal; using Octokit.Tests.Helpers; using Xunit; @@ -129,7 +131,12 @@ namespace Octokit.Tests.Clients { var data = new NewAuthorization(); var client = Substitute.For(); - client.Put(Args.Uri, Args.Object, Args.String).Returns(_ => { throw new AuthorizationException(); }); + client.Put(Args.Uri, Args.Object, Args.String) + .Returns(_ => + { + throw new AuthorizationException( + new ApiResponse { StatusCode = HttpStatusCode.Unauthorized}); + }); var authEndpoint = new AuthorizationsClient(client); AssertEx.Throws(async () => diff --git a/Octokit.Tests/Exceptions/ApiExceptionTests.cs b/Octokit.Tests/Exceptions/ApiExceptionTests.cs new file mode 100644 index 00000000..ea3e6097 --- /dev/null +++ b/Octokit.Tests/Exceptions/ApiExceptionTests.cs @@ -0,0 +1,93 @@ +using System.IO; +using System.Linq; +using System.Net; +using System.Runtime.Serialization.Formatters.Binary; +using NSubstitute; +using Octokit.Internal; +using Xunit; +using Xunit.Extensions; + +namespace Octokit.Tests.Exceptions +{ + public class ApiExceptionTests + { + public class TheConstructor + { + [Fact] + public void CreatesGitHubErrorFromJsonResponse() + { + var response = new ApiResponse + { + Body = @"{""errors"":[{""code"":""custom"",""field"":""key"",""message"":""key is " + + @"already in use"",""resource"":""PublicKey""}],""message"":""Validation Failed""}", + StatusCode = HttpStatusCode.GatewayTimeout + }; + + var exception = new ApiException(response); + + Assert.Equal("Validation Failed", exception.ApiError.Message); + Assert.Equal("key is already in use", exception.ApiError.Errors.First().Message); + Assert.Equal(HttpStatusCode.GatewayTimeout, exception.StatusCode); + } + + [Theory] + [InlineData("")] + [InlineData(null)] + [InlineData("{{{{{")] + public void CreatesGitHubErrorIfResponseMessageIsNotValidJson(string responseContent) + { + var response = new ApiResponse + { + Body = responseContent, + StatusCode = HttpStatusCode.GatewayTimeout + }; + + var exception = new ApiException(response); + + Assert.Equal(responseContent, exception.ApiError.Message); + Assert.Equal(HttpStatusCode.GatewayTimeout, exception.StatusCode); + } + + [Fact] + public void CreatesEmptyGitHubErrorWhenResponseBodyIsNull() + { + var response = Substitute.For(); + response.Body.Returns("test"); + + var exception = new ApiException(); + var anotherException = new ApiException(new ApiResponse { Body = "message1" }); + var thirdException = new ApiException(new ApiResponse { Body = "message2" }); + + // It's fine if the message is null when there's no response body as long as this doesn't throw. + Assert.Null(exception.ApiError.Message); + Assert.Equal("message1", anotherException.ApiError.Message); + Assert.Equal("message2", thirdException.ApiError.Message); + } + +#if !NETFX_CORE + [Fact] + public void CanPopulateObjectFromSerializedData() + { + var response = new ApiResponse + { + Body = @"{""errors"":[{""code"":""custom"",""field"":""key"",""message"":""key is " + + @"already in use"",""resource"":""PublicKey""}],""message"":""Validation Failed""}", + StatusCode = (HttpStatusCode)422 + }; + + var exception = new ApiException(response); + + using (var stream = new MemoryStream()) + { + var formatter = new BinaryFormatter(); + formatter.Serialize(stream, exception); + stream.Position = 0; + var deserialized = (ApiException)formatter.Deserialize(stream); + Assert.Equal("Validation Failed", deserialized.ApiError.Message); + Assert.Equal("key is already in use", exception.ApiError.Errors.First().Message); + } + } +#endif + } + } +} diff --git a/Octokit.Tests/Exceptions/ApiValidationExceptionTests.cs b/Octokit.Tests/Exceptions/ApiValidationExceptionTests.cs index c3023c47..f43995d4 100644 --- a/Octokit.Tests/Exceptions/ApiValidationExceptionTests.cs +++ b/Octokit.Tests/Exceptions/ApiValidationExceptionTests.cs @@ -1,10 +1,9 @@ -using System; -using System.IO; +using System.IO; using System.Linq; +using System.Net; using System.Runtime.Serialization.Formatters.Binary; -using NSubstitute; +using Octokit.Internal; using Xunit; -using Xunit.Extensions; namespace Octokit.Tests.Exceptions { @@ -15,67 +14,43 @@ namespace Octokit.Tests.Exceptions [Fact] public void CreatesGitHubErrorFromJsonResponse() { - var response = Substitute.For(); - response.Body.Returns(@"{""errors"":[{""code"":""custom"",""field"":""key"",""message"":""key is " + - @"already in use"",""resource"":""PublicKey""}],""message"":""Validation Failed""}"); + var response = new ApiResponse + { + Body = @"{""errors"":[{""code"":""custom"",""field"":""key"",""message"":""key is " + + @"already in use"",""resource"":""PublicKey""}],""message"":""Validation Failed""}", + StatusCode = (HttpStatusCode)422 + }; var exception = new ApiValidationException(response); - Assert.Equal("Validation Failed", exception.ApiValidationError.Message); - Assert.Equal("key is already in use", exception.ApiValidationError.Errors.First().Message); - } - - [Theory] - [InlineData("")] - [InlineData(null)] - [InlineData("{{{{{")] - public void CreatesGitHubErrorIfResponseMessageIsNotValidJson(string responseContent) - { - var response = Substitute.For(); - response.Body.Returns(responseContent); - - var exception = new ApiValidationException(response); - - Assert.Equal(responseContent, exception.ApiValidationError.Message); - } - - [Fact] - public void CreatesEmptyGitHubErrorWhenResponseBodyIsNull() - { - var response = Substitute.For(); - response.Body.Returns("test"); - - var exception = new ApiValidationException(); - var anotherException = new ApiValidationException("message1"); - var thirdException = new ApiValidationException("message2", new InvalidOperationException()); - - // It's fine if the message is null when there's no response body as long as this doesn't throw. - Assert.Null(exception.ApiValidationError.Message); - Assert.Equal("message1", anotherException.ApiValidationError.Message); - Assert.Equal("message2", thirdException.ApiValidationError.Message); + Assert.Equal("Validation Failed", exception.ApiError.Message); + Assert.Equal("key is already in use", exception.ApiError.Errors.First().Message); } #if !NETFX_CORE [Fact] public void CanPopulateObjectFromSerializedData() { - var response = Substitute.For(); - response.Body.Returns(@"{""errors"":[{""code"":""custom"",""field"":""key"",""message"":""key is " + - @"already in use"",""resource"":""PublicKey""}],""message"":""Validation Failed""}"); + var response = new ApiResponse + { + Body = @"{""errors"":[{""code"":""custom"",""field"":""key"",""message"":""key is " + + @"already in use"",""resource"":""PublicKey""}],""message"":""Validation Failed""}", + StatusCode = (HttpStatusCode)422 + }; var exception = new ApiValidationException(response); - + using (var stream = new MemoryStream()) { var formatter = new BinaryFormatter(); formatter.Serialize(stream, exception); stream.Position = 0; var deserialized = (ApiValidationException)formatter.Deserialize(stream); - Assert.Equal("Validation Failed", deserialized.ApiValidationError.Message); - Assert.Equal("key is already in use", exception.ApiValidationError.Errors.First().Message); + Assert.Equal("Validation Failed", deserialized.ApiError.Message); + Assert.Equal("key is already in use", exception.ApiError.Errors.First().Message); } } #endif } } -} \ No newline at end of file +} diff --git a/Octokit.Tests/Exceptions/ForbiddenExceptionTests.cs b/Octokit.Tests/Exceptions/ForbiddenExceptionTests.cs new file mode 100644 index 00000000..ee694fcd --- /dev/null +++ b/Octokit.Tests/Exceptions/ForbiddenExceptionTests.cs @@ -0,0 +1,23 @@ +using System.Net; +using Octokit.Internal; +using Xunit; + +namespace Octokit.Tests.Exceptions +{ + public class ForbiddenExceptionTests + { + public class TheConstructor + { + [Fact] + public void IdentifiesMaxLoginAttepmtsExceededReason() + { + const string responseBody = "{\"message\":\"YOU SHALL NOT PASS!\"," + + "\"documentation_url\":\"http://developer.github.com/v3\"}"; + var response = new ApiResponse { Body = responseBody, StatusCode = HttpStatusCode.Forbidden }; + var forbiddenException = new ForbiddenException(response); + + Assert.Equal("YOU SHALL NOT PASS!", forbiddenException.ApiError.Message); + } + } + } +} diff --git a/Octokit.Tests/Exceptions/RateLimitExceededExceptionTests.cs b/Octokit.Tests/Exceptions/RateLimitExceededExceptionTests.cs new file mode 100644 index 00000000..266e3c78 --- /dev/null +++ b/Octokit.Tests/Exceptions/RateLimitExceededExceptionTests.cs @@ -0,0 +1,100 @@ +using System; +using System.Globalization; +using System.IO; +using System.Net; +using System.Runtime.Serialization.Formatters.Binary; +using Octokit.Internal; +using Xunit; + +namespace Octokit.Tests.Exceptions +{ + public class RateLimitExceededExceptionTests + { + public class TheConstructor + { + [Fact] + public void ParsesRateLimitsFromHeaders() + { + var response = new ApiResponse { StatusCode = HttpStatusCode.Forbidden }; + response.Headers.Add("X-RateLimit-Limit", "100"); + response.Headers.Add("X-RateLimit-Remaining", "42"); + response.Headers.Add("X-RateLimit-Reset", "1372700873"); + var exception = new RateLimitExceededException(response); + + Assert.Equal(HttpStatusCode.Forbidden, exception.StatusCode); + Assert.Equal(100, exception.Limit); + Assert.Equal(42, exception.Remaining); + var expectedReset = DateTimeOffset.ParseExact( + "Mon 01 Jul 2013 5:47:53 PM -00:00", + "ddd dd MMM yyyy h:mm:ss tt zzz", + CultureInfo.InvariantCulture); + Assert.Equal(expectedReset, exception.Reset); + } + + [Fact] + public void HandlesInvalidHeaderValues() + { + var response = new ApiResponse { StatusCode = HttpStatusCode.Forbidden }; + response.Headers.Add("X-RateLimit-Limit", "XXX"); + response.Headers.Add("X-RateLimit-Remaining", "XXXX"); + response.Headers.Add("X-RateLimit-Reset", "XXXX"); + var exception = new RateLimitExceededException(response); + + Assert.Equal(HttpStatusCode.Forbidden, exception.StatusCode); + Assert.Equal(0, exception.Limit); + Assert.Equal(0, exception.Remaining); + var expectedReset = DateTimeOffset.ParseExact( + "Thu 01 Jan 1970 0:00:00 AM -00:00", + "ddd dd MMM yyyy h:mm:ss tt zzz", + CultureInfo.InvariantCulture); + Assert.Equal(expectedReset, exception.Reset); + } + + [Fact] + public void HandlesMissingHeaderValues() + { + var response = new ApiResponse { StatusCode = HttpStatusCode.Forbidden }; + var exception = new RateLimitExceededException(response); + + Assert.Equal(HttpStatusCode.Forbidden, exception.StatusCode); + Assert.Equal(0, exception.Limit); + Assert.Equal(0, exception.Remaining); + var expectedReset = DateTimeOffset.ParseExact( + "Thu 01 Jan 1970 0:00:00 AM -00:00", + "ddd dd MMM yyyy h:mm:ss tt zzz", + CultureInfo.InvariantCulture); + Assert.Equal(expectedReset, exception.Reset); + } + +#if !NETFX_CORE + [Fact] + public void CanPopulateObjectFromSerializedData() + { + var response = new ApiResponse { StatusCode = HttpStatusCode.Forbidden }; + response.Headers.Add("X-RateLimit-Limit", "100"); + response.Headers.Add("X-RateLimit-Remaining", "42"); + response.Headers.Add("X-RateLimit-Reset", "1372700873"); + + var exception = new RateLimitExceededException(response); + + using (var stream = new MemoryStream()) + { + var formatter = new BinaryFormatter(); + formatter.Serialize(stream, exception); + stream.Position = 0; + var deserialized = (RateLimitExceededException)formatter.Deserialize(stream); + + Assert.Equal(HttpStatusCode.Forbidden, deserialized.StatusCode); + Assert.Equal(100, deserialized.Limit); + Assert.Equal(42, deserialized.Remaining); + var expectedReset = DateTimeOffset.ParseExact( + "Mon 01 Jul 2013 5:47:53 PM -00:00", + "ddd dd MMM yyyy h:mm:ss tt zzz", + CultureInfo.InvariantCulture); + Assert.Equal(expectedReset, deserialized.Reset); + } + } +#endif + } + } +} diff --git a/Octokit.Tests/Http/ConnectionTests.cs b/Octokit.Tests/Http/ConnectionTests.cs index 04cf4e20..263d048f 100644 --- a/Octokit.Tests/Http/ConnectionTests.cs +++ b/Octokit.Tests/Http/ConnectionTests.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Generic; using System.IO; using System.Linq; using System.Net; @@ -102,9 +101,7 @@ namespace Octokit.Tests.Http var exception = await AssertEx.Throws( async () => await connection.GetAsync(new Uri("/endpoint", UriKind.Relative))); - - Assert.Equal("You must be authenticated to call this method. Either supply a login/password or an " + - "oauth token.", exception.Message); + Assert.NotNull(exception); } [Theory] @@ -128,8 +125,7 @@ namespace Octokit.Tests.Http var exception = await AssertEx.Throws( async () => await connection.GetAsync(new Uri("/endpoint", UriKind.Relative))); - Assert.Equal("You must be authenticated to call this method. Either supply a login/password or an " + - "oauth token.", exception.Message); + Assert.Equal(HttpStatusCode.Unauthorized, exception.StatusCode); } [Theory] @@ -160,7 +156,6 @@ namespace Octokit.Tests.Http var exception = await AssertEx.Throws( async () => await connection.GetAsync(new Uri("/endpoint", UriKind.Relative))); - Assert.Equal("Two-factor authentication required", exception.Message); Assert.Equal(expectedFactorType, exception.TwoFactorType); } @@ -185,7 +180,77 @@ namespace Octokit.Tests.Http async () => await connection.GetAsync(new Uri("/endpoint", UriKind.Relative))); Assert.Equal("Validation Failed", exception.Message); - Assert.Equal("key is already in use", exception.ApiValidationError.Errors[0].Message); + Assert.Equal("key is already in use", exception.ApiError.Errors[0].Message); + } + + [Fact] + public async Task ThrowsRateLimitExceededExceptionForForbidderResponse() + { + var httpClient = Substitute.For(); + IResponse response = new ApiResponse + { + StatusCode = HttpStatusCode.Forbidden, + Body = "{\"message\":\"API rate limit exceeded. " + + "See http://developer.github.com/v3/#rate-limiting for details.\"}" + }; + httpClient.Send(Args.Request).Returns(Task.FromResult(response)); + var connection = new Connection("Test Runner User Agent", + ExampleUri, + Substitute.For(), + httpClient, + Substitute.For()); + + var exception = await AssertEx.Throws( + async () => await connection.GetAsync(new Uri("/endpoint", UriKind.Relative))); + + Assert.Equal("API rate limit exceeded. See http://developer.github.com/v3/#rate-limiting for details.", + exception.Message); + } + + [Fact] + public async Task ThrowsLoginAttemptsExceededExceptionForForbiddenResponse() + { + var httpClient = Substitute.For(); + IResponse response = new ApiResponse + { + StatusCode = HttpStatusCode.Forbidden, + Body = "{\"message\":\"Maximum number of login attempts exceeded\"," + + "\"documentation_url\":\"http://developer.github.com/v3\"}" + }; + httpClient.Send(Args.Request).Returns(Task.FromResult(response)); + var connection = new Connection("Test Runner User Agent", + ExampleUri, + Substitute.For(), + httpClient, + Substitute.For()); + + var exception = await AssertEx.Throws( + async () => await connection.GetAsync(new Uri("/endpoint", UriKind.Relative))); + + Assert.Equal("Maximum number of login attempts exceeded", exception.Message); + Assert.Equal("http://developer.github.com/v3", exception.ApiError.DocumentationUrl); + } + + [Fact] + public async Task ThrowsForbiddenExceptionForUnknownForbiddenResponse() + { + var httpClient = Substitute.For(); + IResponse response = new ApiResponse + { + StatusCode = HttpStatusCode.Forbidden, + Body = "YOU SHALL NOT PASS!" + }; + httpClient.Send(Args.Request).Returns(Task.FromResult(response)); + var connection = new Connection("Test Runner User Agent", + ExampleUri, + Substitute.For(), + httpClient, + Substitute.For()); + + var exception = await AssertEx.Throws( + async () => await connection.GetAsync(new Uri("/endpoint", UriKind.Relative))); + + Assert.Equal("YOU SHALL NOT PASS!", exception.Message); } } diff --git a/Octokit.Tests/Octokit.Tests.csproj b/Octokit.Tests/Octokit.Tests.csproj index ea073d6e..943fbd73 100644 --- a/Octokit.Tests/Octokit.Tests.csproj +++ b/Octokit.Tests/Octokit.Tests.csproj @@ -62,7 +62,10 @@ + + + diff --git a/Octokit.Tests/OctokitRT.Tests.csproj b/Octokit.Tests/OctokitRT.Tests.csproj index b8c80011..24aec30b 100644 --- a/Octokit.Tests/OctokitRT.Tests.csproj +++ b/Octokit.Tests/OctokitRT.Tests.csproj @@ -54,7 +54,10 @@ + + + diff --git a/Octokit.Tests/Reactive/AuthorizationExtensionsTests.cs b/Octokit.Tests/Reactive/AuthorizationExtensionsTests.cs index 1027008e..0e3a9364 100644 --- a/Octokit.Tests/Reactive/AuthorizationExtensionsTests.cs +++ b/Octokit.Tests/Reactive/AuthorizationExtensionsTests.cs @@ -3,7 +3,6 @@ using System.Reactive.Linq; using System.Threading.Tasks; using NSubstitute; using Octokit.Reactive; -using Octokit.Tests.Helpers; using Xunit; namespace Octokit.Tests.Reactive @@ -15,7 +14,7 @@ namespace Octokit.Tests.Reactive [Fact] public async Task UsesCallbackToRetrievTwoFactorCode() { - var firstResponse = new TwoFactorRequiredException("doh", TwoFactorType.AuthenticatorApp); + var firstResponse = new TwoFactorRequiredException(TwoFactorType.AuthenticatorApp); var twoFactorChallengeResult = new TwoFactorChallengeResult("two-factor-code"); var secondResponse = new Authorization {Token = "OAUTHSECRET"}; @@ -46,7 +45,7 @@ namespace Octokit.Tests.Reactive [Fact] public async Task RetriesWhenResendRequested() { - var firstResponse = new TwoFactorRequiredException("doh", TwoFactorType.AuthenticatorApp); + var firstResponse = new TwoFactorRequiredException(TwoFactorType.AuthenticatorApp); var challengeResults = new Queue(new[] { TwoFactorChallengeResult.RequestResendCode, diff --git a/Octokit/Clients/AuthorizationsClient.cs b/Octokit/Clients/AuthorizationsClient.cs index 5bfbf28d..455baa94 100644 --- a/Octokit/Clients/AuthorizationsClient.cs +++ b/Octokit/Clients/AuthorizationsClient.cs @@ -131,7 +131,7 @@ namespace Octokit } catch (AuthorizationException e) { - throw new TwoFactorChallengeFailedException("Two-Factor Authentication code is not valid", e); + throw new TwoFactorChallengeFailedException(e); } } diff --git a/Octokit/Exceptions/ApiException.cs b/Octokit/Exceptions/ApiException.cs index 9085c0a3..96ccf9f7 100644 --- a/Octokit/Exceptions/ApiException.cs +++ b/Octokit/Exceptions/ApiException.cs @@ -1,58 +1,92 @@ using System; +using System.Diagnostics.CodeAnalysis; using System.Net; using System.Runtime.Serialization; +using Octokit.Internal; namespace Octokit { #if !NETFX_CORE [Serializable] #endif + [SuppressMessage("Microsoft.Design", "CA1032:ImplementStandardExceptionConstructors", + Justification = "These exceptions are specific to the GitHub API and not general purpose exceptions")] public class ApiException : Exception { - public ApiException() + // This needs to be hard-coded for translating GitHub error messages. + static readonly IJsonSerializer _jsonSerializer = new SimpleJsonSerializer(); + + public ApiException() : this(new ApiResponse()) { } - public ApiException(HttpStatusCode statusCode) - { - StatusCode = statusCode; - } - - public ApiException(string message) : base(message) + public ApiException(IResponse response) + : this(response, null) { } - public ApiException(string message, Exception innerException) - : base(message, innerException) + public ApiException(IResponse response, Exception innerException) + : base(null, innerException) { + Ensure.ArgumentNotNull(response, "response"); + + StatusCode = response.StatusCode; + ApiError = GetApiErrorFromExceptionMessage(response); } - public ApiException(string message, Exception innerException, HttpStatusCode statusCode) - : base(message, innerException) + public override string Message { - StatusCode = statusCode; - } - - public ApiException(string message, HttpStatusCode statusCode) : base(message) - { - StatusCode = statusCode; + get { return ApiErrorMessageSafe; } } public HttpStatusCode StatusCode { get; private set; } + public ApiError ApiError { get; private set; } + + static ApiError GetApiErrorFromExceptionMessage(IResponse response) + { + string responseBody = response != null ? response.Body : null; + return GetApiErrorFromExceptionMessage(responseBody); + } + + [SuppressMessage("Microsoft.Design", "CA1031:DoNotCatchGeneralExceptionTypes")] + static ApiError GetApiErrorFromExceptionMessage(string responseContent) + { + try + { + if (responseContent != null) + return _jsonSerializer.Deserialize(responseContent) ?? new ApiError { Message = responseContent }; + } + catch (Exception) + { + } + + return new ApiError { Message = responseContent }; + } + #if !NETFX_CORE protected ApiException(SerializationInfo info, StreamingContext context) : base(info, context) { if (info == null) return; StatusCode = (HttpStatusCode)(info.GetInt32("HttpStatusCode")); + ApiError = (ApiError)(info.GetValue("ApiError", typeof(ApiError))); } public override void GetObjectData(SerializationInfo info, StreamingContext context) { base.GetObjectData(info, context); info.AddValue("HttpStatusCode", StatusCode); + info.AddValue("ApiError", ApiError); } #endif + + protected string ApiErrorMessageSafe + { + get + { + return ApiError != null ? ApiError.Message : null; + } + } } } diff --git a/Octokit/Exceptions/ApiValidationException.cs b/Octokit/Exceptions/ApiValidationException.cs index f3f49cff..625ef268 100644 --- a/Octokit/Exceptions/ApiValidationException.cs +++ b/Octokit/Exceptions/ApiValidationException.cs @@ -1,5 +1,7 @@ using System; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; +using System.Net; using System.Runtime.Serialization; using Octokit.Internal; @@ -11,23 +13,11 @@ namespace Octokit #if !NETFX_CORE [Serializable] #endif + [SuppressMessage("Microsoft.Design", "CA1032:ImplementStandardExceptionConstructors", + Justification = "These exceptions are specific to the GitHub API and not general purpose exceptions")] public class ApiValidationException : ApiException { - // This needs to be hard-coded for translating GitHub error messages. - static readonly IJsonSerializer _jsonSerializer = new SimpleJsonSerializer(); - - public ApiValidationException() - : this(new ApiError(), null) - { - } - - public ApiValidationException(string message) - : this(new ApiError { Message = message }, null) - { - } - - public ApiValidationException(string message, Exception innerException) - : this(new ApiError { Message = message }, innerException) + public ApiValidationException() : base(new ApiResponse { StatusCode = (HttpStatusCode)422}) { } @@ -37,55 +27,17 @@ namespace Octokit } public ApiValidationException(IResponse response, Exception innerException) - : this(GetApiErrorFromExceptionMessage(response), innerException) + : base(response, innerException) { - } - - protected ApiValidationException(ApiError apiValidationError, Exception innerException) - : base(apiValidationError != null ? apiValidationError.Message : "An API error occurred", innerException) - { - ApiValidationError = apiValidationError; + Debug.Assert(response != null && response.StatusCode == (HttpStatusCode)422, + "ApiValidationException created with wrong status code"); } #if !NETFX_CORE protected ApiValidationException(SerializationInfo info, StreamingContext context) : base(info, context) { - ApiValidationError = GetApiErrorFromExceptionMessage(info.GetString("ApiValidationError")); - } - - public override void GetObjectData(SerializationInfo info, StreamingContext context) - { - base.GetObjectData(info, context); - - info.AddValue("ApiValidationError", _jsonSerializer.Serialize(ApiValidationError)); } #endif - - public ApiError ApiValidationError { get; private set; } - - static ApiError GetApiErrorFromExceptionMessage(IResponse response) - { - string responseBody = response != null ? response.Body : null; - return GetApiErrorFromExceptionMessage(responseBody); - } - - [SuppressMessage("Microsoft.Design", "CA1031:DoNotCatchGeneralExceptionTypes")] - static ApiError GetApiErrorFromExceptionMessage(string responseContent) - { - try - { - if (responseContent != null) - { - return _jsonSerializer.Deserialize(responseContent) - ?? new ApiError { Message = responseContent }; - } - } - catch (Exception) - { - } - - return new ApiError { Message = responseContent }; - } } } diff --git a/Octokit/Exceptions/AuthorizationException.cs b/Octokit/Exceptions/AuthorizationException.cs index b9888102..9ae9804d 100644 --- a/Octokit/Exceptions/AuthorizationException.cs +++ b/Octokit/Exceptions/AuthorizationException.cs @@ -1,6 +1,9 @@ using System; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Net; using System.Runtime.Serialization; +using Octokit.Internal; namespace Octokit { @@ -10,20 +13,24 @@ namespace Octokit #if !NETFX_CORE [Serializable] #endif + [SuppressMessage("Microsoft.Design", "CA1032:ImplementStandardExceptionConstructors", + Justification = "These exceptions are specific to the GitHub API and not general purpose exceptions")] public class AuthorizationException : ApiException { - public AuthorizationException() + public AuthorizationException() : base(new ApiResponse { StatusCode = HttpStatusCode.Unauthorized }) { } - public AuthorizationException(string message) - : base(message, null, HttpStatusCode.Unauthorized) + public AuthorizationException(IResponse response) + : this(response, null) { } - public AuthorizationException(string message, Exception innerException) - : base(message, innerException, HttpStatusCode.Unauthorized) + public AuthorizationException(IResponse response, Exception innerException) + : base(response, innerException) { + Debug.Assert(response != null && response.StatusCode == HttpStatusCode.Unauthorized, + "AuthorizationException created with wrong status code"); } #if !NETFX_CORE @@ -33,4 +40,4 @@ namespace Octokit } #endif } -} \ No newline at end of file +} diff --git a/Octokit/Exceptions/ForbiddenException.cs b/Octokit/Exceptions/ForbiddenException.cs new file mode 100644 index 00000000..c99af59b --- /dev/null +++ b/Octokit/Exceptions/ForbiddenException.cs @@ -0,0 +1,35 @@ +using System; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Net; +using System.Runtime.Serialization; + +namespace Octokit +{ +#if !NETFX_CORE + [Serializable] +#endif + [SuppressMessage("Microsoft.Design", "CA1032:ImplementStandardExceptionConstructors", + Justification = "These exceptions are specific to the GitHub API and not general purpose exceptions")] + public class ForbiddenException : ApiException + { + public ForbiddenException(IResponse response) : this(response, null) + { + } + + public ForbiddenException(IResponse response, Exception innerException) + : base(response, innerException) + { + Debug.Assert(response != null && response.StatusCode == HttpStatusCode.Forbidden, + "ForbiddenException created with wrong status code"); + } + + +#if !NETFX_CORE + protected ForbiddenException(SerializationInfo info, StreamingContext context) + : base(info, context) + { + } +#endif + } +} diff --git a/Octokit/Exceptions/LoginAttemptsExceededException.cs b/Octokit/Exceptions/LoginAttemptsExceededException.cs new file mode 100644 index 00000000..57ced2bd --- /dev/null +++ b/Octokit/Exceptions/LoginAttemptsExceededException.cs @@ -0,0 +1,31 @@ +using System; +using System.Diagnostics.CodeAnalysis; +using System.Runtime.Serialization; + +namespace Octokit +{ +#if !NETFX_CORE + [Serializable] +#endif + [SuppressMessage("Microsoft.Design", "CA1032:ImplementStandardExceptionConstructors", + Justification = "These exceptions are specific to the GitHub API and not general purpose exceptions")] + public class LoginAttemptsExceededException : ForbiddenException + { + public LoginAttemptsExceededException(IResponse response) : base(response) + { + } + + public LoginAttemptsExceededException(IResponse response, Exception innerException) + : base(response, innerException) + { + } + +#if !NETFX_CORE + protected LoginAttemptsExceededException(SerializationInfo info, StreamingContext context) + : base(info, context) + { + } +#endif + + } +} diff --git a/Octokit/Exceptions/RateLimitExceededException.cs b/Octokit/Exceptions/RateLimitExceededException.cs new file mode 100644 index 00000000..4919288d --- /dev/null +++ b/Octokit/Exceptions/RateLimitExceededException.cs @@ -0,0 +1,89 @@ +using System; +using System.Diagnostics.CodeAnalysis; +using System.Runtime.Serialization; + +namespace Octokit +{ + /// + /// Exception thrown when GitHub API Rate limits are exceeded. + /// + /// + /// + /// For requests using Basic Authentication or OAuth, you can make up to 5,000 requests per hour. For + /// unauthenticated requests, the rate limit allows you to make up to 60 requests per hour. + /// + /// See http://developer.github.com/v3/#rate-limiting for more details. + /// +#if !NETFX_CORE + [Serializable] +#endif + [SuppressMessage("Microsoft.Design", "CA1032:ImplementStandardExceptionConstructors", + Justification = "These exceptions are specific to the GitHub API and not general purpose exceptions")] + public class RateLimitExceededException : ForbiddenException + { + readonly int _resetUnixEpochSeconds; + + public RateLimitExceededException(IResponse response) : this(response, null) + { + } + + public RateLimitExceededException(IResponse response, Exception innerException) : base(response, innerException) + { + Ensure.ArgumentNotNull(response, "response"); + + Limit = ToInt32Safe(response, "X-RateLimit-Limit"); + Remaining = ToInt32Safe(response, "X-RateLimit-Remaining"); + _resetUnixEpochSeconds = ToInt32Safe(response, "X-RateLimit-Reset"); + Reset = FromUnixTime(_resetUnixEpochSeconds); + } + + /// + /// The maximum number of requests that the consumer is permitted to make per hour. + /// + public int Limit { get; private set; } + + /// + /// The number of requests remaining in the current rate limit window. + /// + public int Remaining { get; private set; } + + /// + /// The time at which the current rate limit window resets + /// + public DateTimeOffset Reset { get; private set; } + +#if !NETFX_CORE + protected RateLimitExceededException(SerializationInfo info, StreamingContext context) + : base(info, context) + { + Limit = info.GetInt32("Limit"); + Remaining = info.GetInt32("Remaining"); + _resetUnixEpochSeconds = info.GetInt32("Reset"); + Reset = FromUnixTime(_resetUnixEpochSeconds); + } + + public override void GetObjectData(SerializationInfo info, StreamingContext context) + { + base.GetObjectData(info, context); + info.AddValue("Limit", Limit); + info.AddValue("Remaining", Remaining); + info.AddValue("Reset", _resetUnixEpochSeconds); + } +#endif + + static DateTimeOffset FromUnixTime(long unixTime) + { + var epoch = new DateTimeOffset(1970, 1, 1, 0, 0, 0, TimeSpan.Zero); + return epoch.AddSeconds(unixTime); + } + + static int ToInt32Safe(IResponse response, string key) + { + string value; + int result; + return !response.Headers.TryGetValue(key, out value) || value == null || !int.TryParse(value, out result) + ? 0 + : result; + } + } +} diff --git a/Octokit/Exceptions/TwoFactorChallengeFailedException.cs b/Octokit/Exceptions/TwoFactorChallengeFailedException.cs index d870590f..d25f85e3 100644 --- a/Octokit/Exceptions/TwoFactorChallengeFailedException.cs +++ b/Octokit/Exceptions/TwoFactorChallengeFailedException.cs @@ -1,23 +1,25 @@ using System; +using System.Diagnostics.CodeAnalysis; +using System.Net; using System.Runtime.Serialization; +using Octokit.Internal; namespace Octokit { #if !NETFX_CORE [Serializable] #endif + [SuppressMessage("Microsoft.Design", "CA1032:ImplementStandardExceptionConstructors", + Justification = "These exceptions are specific to the GitHub API and not general purpose exceptions")] public class TwoFactorChallengeFailedException : AuthorizationException { - public TwoFactorChallengeFailedException() + public TwoFactorChallengeFailedException() : + base(new ApiResponse { StatusCode = HttpStatusCode.Unauthorized }) { } - public TwoFactorChallengeFailedException(string message) : base(message) - { - } - - public TwoFactorChallengeFailedException(string message, Exception innerException) - : base(message, innerException) + public TwoFactorChallengeFailedException(Exception innerException) + : base(new ApiResponse { StatusCode = HttpStatusCode.Unauthorized }, innerException) { } diff --git a/Octokit/Exceptions/TwoFactorRequiredException.cs b/Octokit/Exceptions/TwoFactorRequiredException.cs index b2b83585..f256f028 100644 --- a/Octokit/Exceptions/TwoFactorRequiredException.cs +++ b/Octokit/Exceptions/TwoFactorRequiredException.cs @@ -1,29 +1,33 @@ using System; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; +using System.Net; using System.Runtime.Serialization; +using Octokit.Internal; namespace Octokit { #if !NETFX_CORE [Serializable] #endif + [SuppressMessage("Microsoft.Design", "CA1032:ImplementStandardExceptionConstructors", + Justification = "These exceptions are specific to the GitHub API and not general purpose exceptions")] public class TwoFactorRequiredException : AuthorizationException { - public TwoFactorRequiredException() + public TwoFactorRequiredException() : this(TwoFactorType.None) { } - public TwoFactorRequiredException(string message) : base(message) + public TwoFactorRequiredException(TwoFactorType twoFactorType) + : this(new ApiResponse { StatusCode = HttpStatusCode.Unauthorized}, twoFactorType) { } - public TwoFactorRequiredException(string message, Exception innerException) : base(message, innerException) + public TwoFactorRequiredException(IResponse response, TwoFactorType twoFactorType) : base(response) { - } + Debug.Assert(response != null && response.StatusCode == HttpStatusCode.Unauthorized, + "TwoFactorRequiredException status code should be 401"); - public TwoFactorRequiredException(string message, TwoFactorType twoFactorType) - : base(message) - { TwoFactorType = twoFactorType; } diff --git a/Octokit/Http/Connection.cs b/Octokit/Http/Connection.cs index a53dcd54..1a88410d 100644 --- a/Octokit/Http/Connection.cs +++ b/Octokit/Http/Connection.cs @@ -13,23 +13,23 @@ namespace Octokit // ensure it goes through there. :) public class Connection : IConnection { - static readonly Uri defaultGitHubApiUrl = new Uri("https://api.github.com/"); - static readonly ICredentialStore anonymousCredentials = new InMemoryCredentialStore(Credentials.Anonymous); + static readonly Uri _defaultGitHubApiUrl = new Uri("https://api.github.com/"); + static readonly ICredentialStore _anonymousCredentials = new InMemoryCredentialStore(Credentials.Anonymous); readonly Authenticator _authenticator; readonly IHttpClient _httpClient; readonly JsonHttpPipeline _jsonPipeline; readonly ApiInfoParser _apiInfoParser; - public Connection(string userAgent) : this(userAgent, defaultGitHubApiUrl, anonymousCredentials) + public Connection(string userAgent) : this(userAgent, _defaultGitHubApiUrl, _anonymousCredentials) { } - public Connection(string userAgent, Uri baseAddress) : this(userAgent, baseAddress, anonymousCredentials) + public Connection(string userAgent, Uri baseAddress) : this(userAgent, baseAddress, _anonymousCredentials) { } - public Connection(string userAgent, ICredentialStore credentialStore) : this(userAgent, defaultGitHubApiUrl, credentialStore) + public Connection(string userAgent, ICredentialStore credentialStore) : this(userAgent, _defaultGitHubApiUrl, credentialStore) { } @@ -58,8 +58,8 @@ namespace Octokit if (!baseAddress.IsAbsoluteUri) { throw new ArgumentException( - String.Format(CultureInfo.InvariantCulture,"The base address '{0}' must be an absolute URI", - baseAddress), "baseAddress"); + String.Format(CultureInfo.InvariantCulture, "The base address '{0}' must be an absolute URI", + baseAddress), "baseAddress"); } UserAgent = userAgent; @@ -94,7 +94,6 @@ namespace Octokit Ensure.ArgumentNotNull(uri, "uri"); Ensure.ArgumentNotNull(body, "body"); - return await SendData(uri, HttpVerb.Patch, body, null, null); } @@ -125,13 +124,13 @@ namespace Octokit Uri uri, HttpMethod method, object body, - string accepts, + string accepts, string contentType, string twoFactorAuthenticationCode = null - ) + ) { Ensure.ArgumentNotNull(uri, "uri"); - + var request = new Request { Method = method, @@ -162,7 +161,7 @@ namespace Octokit public async Task DeleteAsync(Uri uri) { Ensure.ArgumentNotNull(uri, "uri"); - + await Run(new Request { Method = HttpMethod.Delete, @@ -230,39 +229,48 @@ namespace Octokit return response; } + static readonly Dictionary> _httpExceptionMap = + new Dictionary> + { + { HttpStatusCode.Unauthorized, GetExceptionForUnauthorized }, + { HttpStatusCode.Forbidden, GetExceptionForForbidden }, + { (HttpStatusCode)422, response => new ApiValidationException(response) } + }; + static void HandleErrors(IResponse response) { - if (response.StatusCode == HttpStatusCode.Unauthorized) - { - var twoFactorType = ParseTwoFactorType(response); - throw twoFactorType == TwoFactorType.None - ? new AuthorizationException("You must be authenticated to call this method. Either supply a " + - "login/password or an oauth token.") - : new TwoFactorRequiredException("Two-factor authentication required", twoFactorType); - } - - if (response.StatusCode == HttpStatusCode.Forbidden) + Func exceptionFunc; + if (_httpExceptionMap.TryGetValue(response.StatusCode, out exceptionFunc)) { - throw new ApiException("Request Forbidden", response.StatusCode); - } - - if (response.StatusCode == HttpStatusCode.RequestTimeout) - { - throw new ApiException("Request Timed Out", response.StatusCode); - } - - if ((int)response.StatusCode == 422) - { - throw new ApiValidationException(response); + throw exceptionFunc(response); } if ((int)response.StatusCode >= 400) { - throw new ApiException(response.Body, response.StatusCode); + throw new ApiException(response); } } + static Exception GetExceptionForUnauthorized(IResponse response) + { + var twoFactorType = ParseTwoFactorType(response); + + return twoFactorType == TwoFactorType.None + ? new AuthorizationException(response) + : new TwoFactorRequiredException(response, twoFactorType); + } + + static Exception GetExceptionForForbidden(IResponse response) + { + string body = response.Body ?? ""; + return body.Contains("rate limit exceeded") + ? new RateLimitExceededException(response) + : body.Contains("number of login attempts exceeded") + ? new LoginAttemptsExceededException(response) + : new ForbiddenException(response); + } + static TwoFactorType ParseTwoFactorType(IResponse restResponse) { if (restResponse.Headers == null || !restResponse.Headers.Any()) return TwoFactorType.None; diff --git a/Octokit/Models/ApiError.cs b/Octokit/Models/ApiError.cs index 67ad59f2..69204bcf 100644 --- a/Octokit/Models/ApiError.cs +++ b/Octokit/Models/ApiError.cs @@ -1,12 +1,24 @@ +using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; namespace Octokit { +#if !NETFX_CORE + [Serializable] +#endif public class ApiError { + /// + /// The error message + /// public string Message { get; set; } + /// + /// URL to the documentation for this error. + /// + public string DocumentationUrl { get; set; } + // TODO: This ought to be an IReadOnlyList but we need to add support to SimpleJson for that. [SuppressMessage("Microsoft.Usage", "CA2227:CollectionPropertiesShouldBeReadOnly")] public IList Errors { get; set; } diff --git a/Octokit/Models/ApiErrorDetail.cs b/Octokit/Models/ApiErrorDetail.cs index 10d222cc..6622c395 100644 --- a/Octokit/Models/ApiErrorDetail.cs +++ b/Octokit/Models/ApiErrorDetail.cs @@ -1,5 +1,10 @@ +using System; + namespace Octokit { +#if !NETFX_CORE + [Serializable] +#endif public class ApiErrorDetail { public string Message { get; set; } diff --git a/Octokit/Octokit.csproj b/Octokit/Octokit.csproj index e46a60b1..eaa98466 100644 --- a/Octokit/Octokit.csproj +++ b/Octokit/Octokit.csproj @@ -99,6 +99,9 @@ + + + diff --git a/Octokit/OctokitRT.csproj b/Octokit/OctokitRT.csproj index bb7e5729..0579744a 100644 --- a/Octokit/OctokitRT.csproj +++ b/Octokit/OctokitRT.csproj @@ -121,6 +121,9 @@ + + +