Skip to content
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.

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

[WIP] use HttpContext.RequestAborted to support cancel a request #902

Merged
merged 5 commits into from
May 27, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions Directory.Build.props
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<Project>
<PropertyGroup>
<LangVersion>latest</LangVersion>

<RepositoryType>git</RepositoryType>
<RepositoryUrl>https://github.com/ThreeMammals/Ocelot</RepositoryUrl>
<!-- Optional: Publish the repository URL in the built .nupkg (in the NuSpec <Repository> element) -->
Expand Down
22 changes: 16 additions & 6 deletions src/Ocelot/Errors/Middleware/ExceptionHandlerMiddleware.cs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
namespace Ocelot.Errors.Middleware
{
using Configuration;
using System;
using System.Linq;
using System.Threading.Tasks;
using Ocelot.Configuration.Repository;
using Ocelot.Infrastructure.Extensions;
using Ocelot.Infrastructure.RequestData;
using Ocelot.Logging;
using Ocelot.Middleware;

using System;
using System.Linq;
using System.Threading.Tasks;

/// <summary>
/// Catches all unhandled exceptions thrown by middleware, logs and returns a 500
/// </summary>
Expand All @@ -21,7 +21,7 @@ public class ExceptionHandlerMiddleware : OcelotMiddleware

public ExceptionHandlerMiddleware(OcelotRequestDelegate next,
IOcelotLoggerFactory loggerFactory,
IInternalConfigurationRepository configRepo,
IInternalConfigurationRepository configRepo,
IRequestScopedDataRepository repo)
: base(loggerFactory.CreateLogger<ExceptionHandlerMiddleware>())
{
Expand All @@ -34,6 +34,8 @@ public async Task Invoke(DownstreamContext context)
{
try
{
context.HttpContext.RequestAborted.ThrowIfCancellationRequested();

//try and get the global request id and set it for logs...
//should this basically be immutable per request...i guess it should!
//first thing is get config
Expand All @@ -52,14 +54,22 @@ public async Task Invoke(DownstreamContext context)

await _next.Invoke(context);
}
catch (OperationCanceledException e) when (context.HttpContext.RequestAborted.IsCancellationRequested)
{
Logger.LogDebug("operation canceled");
if (!context.HttpContext.Response.HasStarted)
{
context.HttpContext.Response.StatusCode = 499;
}
}
catch (Exception e)
{
Logger.LogDebug("error calling middleware");

var message = CreateMessage(context, e);

Logger.LogError(message, e);

SetInternalServerErrorOnResponse(context);
}

Expand Down
77 changes: 39 additions & 38 deletions src/Ocelot/Errors/OcelotErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,42 +2,43 @@
{
public enum OcelotErrorCode
{
UnauthenticatedError,
UnknownError,
DownstreampathTemplateAlreadyUsedError,
UnableToFindDownstreamRouteError,
CannotAddDataError,
CannotFindDataError,
UnableToCompleteRequestError,
UnableToCreateAuthenticationHandlerError,
UnsupportedAuthenticationProviderError,
CannotFindClaimError,
ParsingConfigurationHeaderError,
NoInstructionsError,
InstructionNotForClaimsError,
UnauthorizedError,
ClaimValueNotAuthorisedError,
ScopeNotAuthorisedError,
UserDoesNotHaveClaimError,
DownstreamPathTemplateContainsSchemeError,
DownstreamPathNullOrEmptyError,
DownstreamSchemeNullOrEmptyError,
DownstreamHostNullOrEmptyError,
ServicesAreNullError,
ServicesAreEmptyError,
UnableToFindServiceDiscoveryProviderError,
UnableToFindLoadBalancerError,
RequestTimedOutError,
UnableToFindQoSProviderError,
UnmappableRequestError,
RateLimitOptionsError,
PathTemplateDoesntStartWithForwardSlash,
FileValidationFailedError,
UnableToFindDelegatingHandlerProviderError,
CouldNotFindPlaceholderError,
CouldNotFindAggregatorError,
CannotAddPlaceholderError,
CannotRemovePlaceholderError,
QuotaExceededError
UnauthenticatedError = 0,
UnknownError = 1,
DownstreampathTemplateAlreadyUsedError = 2,
UnableToFindDownstreamRouteError = 3,
CannotAddDataError = 4,
CannotFindDataError = 5,
UnableToCompleteRequestError = 6,
UnableToCreateAuthenticationHandlerError = 7,
UnsupportedAuthenticationProviderError = 8,
CannotFindClaimError = 9,
ParsingConfigurationHeaderError = 10,
NoInstructionsError = 11,
InstructionNotForClaimsError = 12,
UnauthorizedError = 13,
ClaimValueNotAuthorisedError = 14,
ScopeNotAuthorisedError = 15,
UserDoesNotHaveClaimError = 16,
DownstreamPathTemplateContainsSchemeError = 17,
DownstreamPathNullOrEmptyError = 18,
DownstreamSchemeNullOrEmptyError = 19,
DownstreamHostNullOrEmptyError = 20,
ServicesAreNullError = 21,
ServicesAreEmptyError = 22,
UnableToFindServiceDiscoveryProviderError = 23,
UnableToFindLoadBalancerError = 24,
RequestTimedOutError = 25,
UnableToFindQoSProviderError = 26,
UnmappableRequestError = 27,
RateLimitOptionsError = 28,
PathTemplateDoesntStartWithForwardSlash = 29,
FileValidationFailedError = 30,
UnableToFindDelegatingHandlerProviderError = 31,
CouldNotFindPlaceholderError = 32,
CouldNotFindAggregatorError = 33,
CannotAddPlaceholderError = 34,
CannotRemovePlaceholderError = 35,
QuotaExceededError = 36,
RequestCanceled = 37,
}
}
}
10 changes: 5 additions & 5 deletions src/Ocelot/Requester/HttpClientHttpRequester.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
using System;
using System.Net.Http;
using System.Threading.Tasks;
using Ocelot.Logging;
using Ocelot.Middleware;
using Ocelot.Responses;
using System;
using System.Net.Http;
using System.Threading.Tasks;

namespace Ocelot.Requester
{
Expand All @@ -16,7 +16,7 @@ public class HttpClientHttpRequester : IHttpRequester

public HttpClientHttpRequester(IOcelotLoggerFactory loggerFactory,
IHttpClientCache cacheHandlers,
IDelegatingHandlerHandlerFactory factory,
IDelegatingHandlerHandlerFactory factory,
IExceptionToErrorMapper mapper)
{
_logger = loggerFactory.CreateLogger<HttpClientHttpRequester>();
Expand All @@ -33,7 +33,7 @@ public async Task<Response<HttpResponseMessage>> GetResponse(DownstreamContext c

try
{
var response = await httpClient.SendAsync(context.DownstreamRequest.ToHttpRequestMessage());
var response = await httpClient.SendAsync(context.DownstreamRequest.ToHttpRequestMessage(), context.HttpContext.RequestAborted);
return new OkResponse<HttpResponseMessage>(response);
}
catch (Exception exception)
Expand Down
5 changes: 3 additions & 2 deletions src/Ocelot/Requester/HttpClientWrapper.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;

namespace Ocelot.Requester
Expand All @@ -15,9 +16,9 @@ public HttpClientWrapper(HttpClient client)
Client = client;
}

public Task<HttpResponseMessage> SendAsync(HttpRequestMessage request)
public Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken = default)
{
return Client.SendAsync(request);
return Client.SendAsync(request, cancellationToken);
}
}
}
9 changes: 7 additions & 2 deletions src/Ocelot/Requester/HttpExeptionToErrorMapper.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
namespace Ocelot.Requester
{
using System;
using System.Collections.Generic;
using Errors;
using Microsoft.Extensions.DependencyInjection;
using System;
using System.Collections.Generic;

public class HttpExeptionToErrorMapper : IExceptionToErrorMapper
{
Expand All @@ -23,6 +23,11 @@ public Error Map(Exception exception)
return _mappers[type](exception);
}

if (type == typeof(OperationCanceledException))
{
return new RequestCanceledError(exception.Message);
}

return new UnableToCompleteRequestError(exception);
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/Ocelot/Requester/IHttpClient.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using System;
using System.Net.Http;
using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;

namespace Ocelot.Requester
Expand All @@ -8,6 +8,6 @@ public interface IHttpClient
{
HttpClient Client { get; }

Task<HttpResponseMessage> SendAsync(HttpRequestMessage request);
Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken = default);
}
}
}
11 changes: 11 additions & 0 deletions src/Ocelot/Requester/RequestCanceledError.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
using Ocelot.Errors;

namespace Ocelot.Requester
{
public class RequestCanceledError : Error
{
public RequestCanceledError(string message) : base(message, OcelotErrorCode.RequestCanceled)
{
}
}
}
14 changes: 11 additions & 3 deletions src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
using Ocelot.Errors;
using System.Collections.Generic;
using System.Linq;
using Ocelot.Errors;

namespace Ocelot.Responder
{
Expand All @@ -13,7 +13,7 @@ public int Map(List<Error> errors)
return 401;
}

if (errors.Any(e => e.Code == OcelotErrorCode.UnauthorizedError
if (errors.Any(e => e.Code == OcelotErrorCode.UnauthorizedError
|| e.Code == OcelotErrorCode.ClaimValueNotAuthorisedError
|| e.Code == OcelotErrorCode.ScopeNotAuthorisedError
|| e.Code == OcelotErrorCode.UserDoesNotHaveClaimError
Expand All @@ -27,6 +27,14 @@ public int Map(List<Error> errors)
return 503;
}

if (errors.Any(e => e.Code == OcelotErrorCode.RequestCanceled))
{
// status code refer to
// https://stackoverflow.com/questions/46234679/what-is-the-correct-http-status-code-for-a-cancelled-request?answertab=votes#tab-top
// https://httpstatuses.com/499
return 499;
}

if (errors.Any(e => e.Code == OcelotErrorCode.UnableToFindDownstreamRouteError))
{
return 404;
Expand All @@ -40,4 +48,4 @@ public int Map(List<Error> errors)
return 404;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ public void should_return_default_error_because_mappers_are_null()
error.ShouldBeOfType<UnableToCompleteRequestError>();
}

[Fact]
public void should_return_request_canceled()
{
var error = _mapper.Map(new OperationCanceledException());

error.ShouldBeOfType<RequestCanceledError>();
}

[Fact]
public void should_return_error_from_mapper()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
using System;
using System.Collections.Generic;
using System.Net;
using Ocelot.Errors;
using Ocelot.Errors;
using Ocelot.Responder;
using Shouldly;
using System;
using System.Collections.Generic;
using System.Net;
using TestStack.BDDfy;
using Xunit;

Expand Down Expand Up @@ -32,7 +32,7 @@ public void should_return_unauthorized(OcelotErrorCode errorCode)
[InlineData(OcelotErrorCode.ClaimValueNotAuthorisedError)]
[InlineData(OcelotErrorCode.ScopeNotAuthorisedError)]
[InlineData(OcelotErrorCode.UnauthorizedError)]
[InlineData(OcelotErrorCode.UserDoesNotHaveClaimError)]
[InlineData(OcelotErrorCode.UserDoesNotHaveClaimError)]
public void should_return_forbidden(OcelotErrorCode errorCode)
{
ShouldMapErrorToStatusCode(errorCode, HttpStatusCode.Forbidden);
Expand Down Expand Up @@ -125,7 +125,7 @@ public void check_we_have_considered_all_errors_in_these_tests()
// If this test fails then it's because the number of error codes has changed.
// You should make the appropriate changes to the test cases here to ensure
// they cover all the error codes, and then modify this assertion.
Enum.GetNames(typeof(OcelotErrorCode)).Length.ShouldBe(37, "Looks like the number of error codes has changed. Do you need to modify ErrorsToHttpStatusCodeMapper?");
Enum.GetNames(typeof(OcelotErrorCode)).Length.ShouldBe(38, "Looks like the number of error codes has changed. Do you need to modify ErrorsToHttpStatusCodeMapper?");
}

private void ShouldMapErrorToStatusCode(OcelotErrorCode errorCode, HttpStatusCode expectedHttpStatusCode)
Expand All @@ -137,7 +137,7 @@ private void ShouldMapErrorsToStatusCode(List<OcelotErrorCode> errorCodes, HttpS
{
var errors = new List<Error>();

foreach(var errorCode in errorCodes)
foreach (var errorCode in errorCodes)
{
errors.Add(new AnyError(errorCode));
}
Expand Down Expand Up @@ -168,4 +168,4 @@ private void ThenTheResponseIsStatusCodeIs(HttpStatusCode expectedCode)
_result.ShouldBe((int)expectedCode);
}
}
}
}