riposte icon indicating copy to clipboard operation
riposte copied to clipboard

ProxyRouterEndpoint should have the capability to immediately return a non-error response instead of proxying

Open nicmunroe opened this issue 8 years ago • 0 comments

ProxyRouterEndpoint currently only allows you to proxy the request, or throw an exception that will get mapped by the error handling system into a response. There needs to be a way to short-circuit the proxy behavior to allow you to intentionally return a non-error response rather than proxy.

Use case: a ProxyRouterEndpoint that either does the normal proxy thing, or returns a 302 redirect instead of proxying.

For now, in order to allow for this feature in a non-breaking way, I think a decent solution would be to add the following method to the ProxyRouterEndpoint class:

public Optional<ResponseInfo<?>> shortCircuitWithImmediateResponse(RequestInfo<?> request) {
    return Optional.empty();
}

And then here in ProxyRouterEndpointExecutionHandler you'd add something like this:


Optional<ResponseInfo<?>> shortCircuitResponse =
    endpointProxyRouter.shortCircuitWithImmediateResponse(requestInfo);

if (shortCircuitResponse != null && shortCircuitResponse.isPresent()) {
    proxyRouterState.cancelRequestStreamingDueToShortCircuitResponse();
    setResponseInfoAndActivatePipelineForResponse(state, shortCircuitResponse.get(), ctx);
    return PipelineContinuationBehavior.DO_NOT_FIRE_CONTINUE_EVENT;
}

Where cancelRequestStreamingDueToShortCircuitResponse() is a new method on ProxyRouterProcessingState that just sets requestStreamingCancelled to true (needed to cause any payload chunks sent by the caller to be aggressively released), and setResponseInfoAndActivatePipelineForResponse(...) is the same-named method copied from NonblockingEndpointExecutionHandler.

Then tests, javadocs, etc. I might be forgetting/missing something important, but those are the broad strokes.

Ultimately if/when we do a major release to Riposte that contains breaking API changes, it might be better to have ProxyRouterEndpoint return a Either<ResponseInfo, DownstreamRequestFirstChunkInfo> or something similar so the short circuit behavior is explicit and obvious rather than being buried in a method you have to override.

nicmunroe avatar Nov 14 '17 23:11 nicmunroe