Client hangs on when trying to create options from empty config

[amorozow42]

Version

4.0.3

Code sample

JsonObject config = new JsonObject();

RabbitMQClient.create(
        vertx,
        new RabbitMQOptions(config)
);

[Yaytay]

I've added a test that does the same to my fork: https://github.com/Yaytay/vertx-rabbitmq-client/blob/master/src/test/java/io/vertx/rabbitmq/RabbitMQClientEmptyJsonTest.java It runs without any issue on Windows, we'll what the CI run says (https://github.com/Yaytay/vertx-rabbitmq-client/actions/runs/738771528).

If that works cleanly then I'm going to need more information to work out why it doesn't work for you.

[Yaytay]

The CI run was also clean, so I'm going to need more information.

[amorozow42]

I was trying to use config() from Verticle while no configuration had been provided. As I know in this case config() returns empty JsonObject. Maybe I'm wrong?

[Yaytay]

Can you provide a full example that fails?

[amorozow42]

I can not reproduce it too now)

public final class NotificationsVerticle extends AbstractVerticle {

  ...

  @Override
  public void start() {

    final RabbitMQClient rabbit = connectToRabbit();

    LOG.info("Notifications service is ready.");
  }

  private RabbitMQClient connectToRabbit() {

    return RabbitMQClient.create(
        vertx,
        new RabbitMQOptions(config())
            .setAutomaticRecoveryEnabled(false)
            .setReconnectAttempts(Integer.MAX_VALUE)
            .setReconnectInterval(500));
  }

    vertx.deployVerticle(
        new NotificationsVerticle()
    );

[amorozow42]

I've got it again. Label "Notifications service is ready" is not printed, while connection is established. Here is full code:

package ru.oksk.smstraffic.notification;

import io.vertx.core.AbstractVerticle;
import io.vertx.core.Future;
import io.vertx.core.Promise;
import io.vertx.core.json.JsonObject;
import io.vertx.rabbitmq.RabbitMQClient;
import io.vertx.rabbitmq.RabbitMQMessage;
import io.vertx.rabbitmq.RabbitMQOptions;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public final class NotificationsVerticle extends AbstractVerticle {

  private static final Logger LOG = LoggerFactory.getLogger(NotificationsVerticle.class);

  private static final String QUEUE_NAME = "subscriber-available";

  private static final String NOTIFICATIONS_ADDRESS = "notifications";

  @Override
  public void start() {

    connectToRabbit()
        .compose(this::listenForNotifications)
        .onSuccess(v -> LOG.info("Notifications service is ready."))
        .onFailure(error -> LOG.error(error.getMessage(), error));
  }

  private Future<RabbitMQClient> connectToRabbit() {

    System.out.println(config().getJsonObject("rabbitmq"));

    final Promise<RabbitMQClient> result = Promise.promise();

    final RabbitMQClient rabbit = RabbitMQClient.create(
        vertx,
        new RabbitMQOptions(config().getJsonObject("rabbitmq"))
            .setAutomaticRecoveryEnabled(false)
            .setReconnectAttempts(Integer.MAX_VALUE)
            .setReconnectInterval(500));

    rabbit
        .start()
        .map(rabbit)
        .onComplete(result);

    return result.future();
  }

  private Future<Void> listenForNotifications(final RabbitMQClient rabbit) {

    final Integer period = config().getInteger("queue_process_period");

    vertx.setPeriodic(period, timer -> {
      rabbit.basicGet(QUEUE_NAME, true, get -> {
        if (get.succeeded()) {
          processMessage(get.result());
        } else {
          LOG.warn("Can not read message from queue.");
        }
      });
    });

    return Future.succeededFuture();
  }

  private void processMessage(final RabbitMQMessage message) {

    final JsonObject body = message.body().toJsonObject();

    LOG.info("New notification: {}.", body.encode());

    vertx.eventBus().publish(NOTIFICATIONS_ADDRESS, body);
  }
}

[amorozow42]

Problem is here:

rabbit
        .start()
        .map(rabbit)
        .onComplete(result);

Method RabbitMQClient::Start doesn't complete promise.

[amorozow42]

This doesn't work too:

    rabbit
        .start(ar -> {
          if (ar.succeeded()) {
            result.complete(rabbit);
          } else {
            result.fail(ar.cause());
          }
        });

[Yaytay]

When I run that in a minimal unit test config() returns an empty object, so config().getJsonObject("rabbitmq") returns null, and the RabbitMQOptions constructor throws a NullPointerException.

[Yaytay]

I think the reason that you are seeing it never succeed is that it is continuously trying to reconnect, and always failing because you haven't provided details of what to connect to (can't be sure without know what your config() is returning). This should show up in the log and is what you've asked it to do with the retry configuration you've explicitly set.

Clearly, if no details about how to connect are given, the client isn't ever going connect no matter how many times it retries. The client could try to identify this and fail the original request, but there are many reasons why the connection could fail and it's not easy to isolate those that can't be fixed by the operator.

[amorozow42]

I have added this line to emphasise that config is correct:

System.out.println(config().getJsonObject("rabbitmq"));

Clearly, if no details about how to connect are given, the client isn't ever going connect no matter how many times it retries. The client could try to identify this and fail the original request, but there are many reasons why the connection could fail and it's not easy to isolate those that can't be fixed by the operator.

Don't see any problems to do this.

[Yaytay]

I have added this line to emphasise that config is correct: System.out.println(config().getJsonObject("rabbitmq"));

But not shown what it prints for you, for me it just prints null.

Clearly, if no details about how to connect are given, the client isn't ever going connect no matter how many times it retries. The client could try to identify this and fail the original request, but there are many reasons why the connection could fail and it's not easy to isolate those that can't be fixed by the operator.

Don't see any problems to do this.

If the config has no address, uri or host it will fail, and could do so explicitly. The problem is that there are many other config errors that would permanently prevent it from connecting and they cannot be detected up front (the port might be wrong, for example). I don't like having special cases for error handling - if you use reconnections you have to check the logs.

[amorozow42]

But not shown what it prints for you, for me it just prints null.

Sorry. It prints:

  "rabbitmq": {
    "host": "172.31.200.50",
    "port": 5672,
    "user": "test",
    "password": "test"
  }
}

I don't like having special cases for error handling - if you use reconnections you have to check the logs.

It is impossible If we have 100 instances of service. We should start reconnecting service only after first success connection attempt. Saying simpler, after calling RabbitMQClient::start() method we can set flag and start connection tracking.

[Yaytay]

Do you know why the connection is failing in this instance?

For my use cases I specifically do want it to start retrying immediately (because I have my client and server are both coming up in a Kubernetes cluster and the client typically starts first). So it sounds like the best solution is another config option to say whether it should start retries immediately or not.

[amorozow42]

Yes, I agree, connection tracking is better to start at once. Can we distinguish client error about wrong configuration?

[Yaytay]

Not easily. We could detect a missing host/uri/address, but there are many other reasons why the connection might be wrong (bad port being the obvious example) - until we try to connect we cannot reliably know whether the config provided will work or not.

You can set a lower number of retries - in my clients I default to 1000, which is enough to give a rabbit container time to start, but will result in my client going down eventually if the server is wrong.

[amorozow42]

We can do one attempt to connect and then stop if wrong config detected.

[Yaytay]

In my kubernetes startup situation the config is "wrong", because the target host does not exist, so one connection attempt will fail. I'm quite happy to add an extra option to disable retries until after a successful connection (for my use case I'd leave that disabled, but you could enable it).

[amorozow42]

Ok, it will be enough., thank you!

PS What about original issue, I've found it was my mistake. https://github.com/eclipse-vertx/vert.x/issues/3890