Skip to content

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

Description

@nicmunroe

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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions