vlingo / xoom-http

The VLINGO XOOM platform SDK for Reactive, scalable, high-throughput, and resilient HTTP server supporting RESTful services running on XOOM LATTICE and XOOM ACTORS.
https://vlingo.io
Mozilla Public License 2.0
51 stars 16 forks source link

Request handler withFailure causes blocked response #48

Closed kbastani closed 4 years ago

kbastani commented 4 years ago

The code block below creates a new Vlingo Boot HTTP server with the request handler containing an OK HTTP body of Hello.

public class VlingoApplication {
    public static void main(String[] args) {
        VlingoServer.create("Vlingo Application",
                ResourceBuilder.resource("Hello", get("/hello")
                        .handle(() -> withFailure(of(Ok, "Hello")))));
    }
}

When using withFailure or throwing a runtime exception in the handler, the server will indefinitely block a response to the client.

curl localhost:8080/hello

This curl request will hang until the server is shut down. At that point the server will not return a reply, but the socket will be closed and the consumer will receive a message...

curl: (52) Empty reply from server

wwerner commented 4 years ago

Just to confirm - I'm seeing this in schemata as well in case an exception is thrown from the persistence actors.

kbastani commented 4 years ago

I've targeted the code in the vlingo-http server where the response is hanging.

abstract class RequestExecutor {

  static Completes<Response> executeRequest(final Supplier<Completes<Response>> executeAction, final ErrorHandler errorHandler, final Logger logger) {
    try {
      return executeAction.get()
        .recoverFrom(ex -> ResourceErrorProcessor.resourceHandlerError(errorHandler, logger, ex));
        // recoverFrom never emits a message that finalizes the state of the actor 
    } catch(Exception ex) {
      return Completes.withFailure(ResourceErrorProcessor.resourceHandlerError(errorHandler, logger, ex));
    }
  }
}

executeAction.get() returns a Supplier<T> that is chained to recoverFrom and is provided a Completes<T> handler. It looks as though the intention here was to attempt to allow the client to provide a remediation strategy to the server before completing.

kbastani commented 4 years ago

I dug deeper and it looks like the workflow for BasicCompletes<T> in vlingo-commons has a workflow issue for withFailure, which depends on failureAction to be set and the executables.actions queue to be empty.

public boolean executeFailureAction() {
    // failureAction is always null, which will keep the server in an infinite while loop
    if (failureAction != null) {
        final Action<T> executeFailureAction = failureAction;
        failureAction = null;
        failed.set(true);
        if (executeFailureAction.isConsumer()) {
            executeFailureAction.asConsumer().accept((T) outcome.get());
        } else {
            outcome.set(executeFailureAction.asFunction().apply((T) outcome.get()));
        }
        return true;
    } else if (parent != null) {
        // bubble up
        return parent.handleFailure(failedValue());
    }

    return false;
}

I think a better workflow for this would be to abstract the entire workflow of withSuccess (which works fine) and to rename it to withResponse. Then remove all the exception handling code that is a part of withFailure. After that, wrap the client request handler method in two helper methods for withFailure and withSuccess which both call withResponse, making the new withFailure call logging out the error before calling withResponse.

VaughnVernon commented 4 years ago

@kbastani I fixed this. It was simpler than I first thought. The problem was in ResourceRequestHandlerActor not handling otherwise() and recoverFrom(). See:

https://github.com/vlingo/vlingo-http/commit/57e83e6fd71c2181b11519498062b34d4c61fb04

If you think it's worth it to clean up BasicCompletes<T> you can do so, but I think it would be best to save it for a reimplementation.