rsocket / rsocket-go

rsocket-go implementation
Apache License 2.0
511 stars 46 forks source link

client connect: rejected setup #102

Closed kuronyago closed 1 year ago

kuronyago commented 3 years ago

If the server responds with a REJECTED_SETUP error, the Start client call does not return an error

Expected Behavior

if server return error (https://github.com/rsocket/rsocket-go/blob/master/examples/echo/echo.go#L55) - the callback should work and an error should be returned to the client

Actual Behavior

the error is returned through an additional mechanism, such as a context, and this happens asynchronously

Steps to Reproduce

https://github.com/kuronyago/rx go run cmd/server/main.go go run cmd/client/main.go

Possible Solution

maybe this is the expected behavior, but then it is not clear how to get a REJECTED_SETUP error in sync, the only option I can think of is time.Sleep, but that is disgusting

Your Environment

jjeffcaii commented 3 years ago

Thanks for reporting!

The REJECTED_SETUP is an error with high priority, when a client sends a SETUP message, server will only send REJECT_SETUP when you return an error, there's no setup response message like SETUP_OK, it causes the setup phase is asynchronous, think about the timeline below:

image

In this case, the client sent three messages and the server rejected the setup after receiving these messages and disconnect the connection, here the problem is how to return the responses for the three requests in client-side?

In current SDK, it will return the REJECT_SETUP for the first request, and a SOCKET_CLOSED error for the rest requests. It's weird, but I tested it with the java SDK:

// Server side
package com.example.rsocket;

import io.rsocket.core.RSocketServer;
import io.rsocket.transport.netty.server.TcpServerTransport;
import reactor.core.publisher.Mono;

public class SetupFailedServer {

  public static void main(String[] args) {
    RSocketServer.create((setup, sendingSocket) -> Mono.error(new RuntimeException("foobar")))
        .bind(TcpServerTransport.create(7878))
        .block()
        .onClose()
        .block();
  }
}
// Client Side
package com.example.rsocket;

import io.rsocket.Payload;
import io.rsocket.RSocket;
import io.rsocket.core.RSocketClient;
import io.rsocket.core.RSocketConnector;
import io.rsocket.transport.netty.client.TcpClientTransport;
import io.rsocket.util.DefaultPayload;
import java.lang.invoke.MethodHandles;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class SetupFailedClient {

  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());

  public static void main(String[] args) {
    RSocket cli = RSocketConnector.create().connect(TcpClientTransport.create(7878)).block();
    log.info("connect success!");
    for (int i = 0; i < 3; i++) {
      try {
        Payload res = cli.requestResponse(DefaultPayload.create("foobar")).block();
        log.info("request success: {}", res);
      } catch (Throwable e) {
        log.error("request failed: {}", e.getMessage());
      }
    }
  }
}

And the execution result is:

21:11:43.972 [main] INFO  c.example.rsocket.SetupFailedClient - connect success!
21:11:44.008 [main] ERROR c.example.rsocket.SetupFailedClient - request failed: foobar
21:11:44.010 [main] ERROR c.example.rsocket.SetupFailedClient - request failed: foobar
21:11:44.010 [main] ERROR c.example.rsocket.SetupFailedClient - request failed: foobar

The different point is java SDK returns three reject setup exception. I think the go SDK should keep the same behavior. I'll fix it.

In short: