[3.2.x] ConnectEvent not triggered if not first event subscribed to (Default EventDispatcher was changed from 3.1.8) (regression?)

[frayien]

Event ConnectEvent is never called if not the first event subcribed to.
I spend all day figuring it out and I can find nothing about it in the docs or anywhere else.
Can confirm it worked for sure before, as I've been updating an old bot. At minimum 3.1.8 worked as intended.
EDIT: tested all versions from 3.2.0 to 3.2.5 and none of them work as intended.
EDIT: I can confirm 3.1.8 works as intended (see code in other).

To Reproduce:

client = DiscordClientBuilder.create(config.discord_token).build();
gateway = client.gateway().setEnabledIntents(IntentSet.all()).login().block();

gateway.getEventDispatcher().on(VoiceStateUpdateEvent.class).subscribe(event -> { System.out.println("Voice state called"); });
gateway.getEventDispatcher().on(ConnectEvent.class).subscribe(event -> { System.out.println("Connect called"); });

Note: example use VoiceStateUpdateEvent but any event do (Tested with ChatInputInteractionEvent, VoiceStateUpdateEvent and DisconnectEvent).
Note: the other event is always called as expected whatever the order.
Note: ReconnectEvent is called as expected.

Expected Behavior:
OnConnectEvent is called

Actual Behavior:
OnConnectEvent never called

Version:
Discord4j 3.2.0 -> 3.2.5

Other:
Workaround, subscribe ConnectEvent first :

client = DiscordClientBuilder.create(config.discord_token).build();
gateway = client.gateway().setEnabledIntents(IntentSet.all()).login().block();

gateway.getEventDispatcher().on(ConnectEvent.class).subscribe(event -> { System.out.println("Connect called"); });
gateway.getEventDispatcher().on(VoiceStateUpdateEvent.class).subscribe(event -> { System.out.println("Voice state called"); });

EDIT: test code for 3.1.8

client = DiscordClientBuilder.create(config.discord_token).build();
gateway = client.login().block();

gateway.getEventDispatcher().on(ConnectEvent.class).subscribe(event -> { System.out.println("Connect called"); });
gateway.getEventDispatcher().on(VoiceStateUpdateEvent.class).subscribe(event -> { System.out.println("Voice state called"); });

[Azn9]

The "ConnectEvent" (as well as the ReadyEvent) is dispatched when a connection is established to the gateway. It it called during the ".login().block()" part, before your listeners are registered.

To run some code when your bot is started, you have two options:

1. Use the "ReadyEvent":
   If you need to listen to this event, register it before the login:

DiscordClientBuilder.create("<token>")
        .build()
        .gateway()
        .setEnabledIntents(IntentSet.all())
        .withGateway(gatewayDiscordClient -> {
            gatewayDiscordClient.on(ReadyEvent.class).subscribe(interactionCreateEvent -> {
                logger.info("ReadyEvent!");
            });

            return gatewayDiscordClient.onDisconnect();
        })
        .block();

2. Don't use an event
   You can asume that if the gateway client is not null (and the .login().block() did not throw an exception), it has successfully connected, so doing something just after calling .login().block() is done right after the login.

[Azn9]

Be aware that using the 1st method will completly block the thread, as the provided code is equivalent to calling client.onDisconnect().block()!

[frayien]

Is this some kind of race condition then ?
Super weird the result seems consistent then ...
I can reproduce with 100% accuracy that it worked in 3.1.8 and not in 3.2.0 onward.
Do you think I got lucky or there is some behaviour change behind the hood ?

[frayien]

Anyway 1. is a good way to avoid undefined behaviour then ! Thanks !

[Azn9]

Nope, the "issue" is using .gateway().

This code works without any issue in 3.2.6-SNAPSHOT:

var client = DiscordClientBuilder.create("<token>")
        .build()
        .login().block();
        
client.on(ReadyEvent.class).subscribe(readyEvent -> {
    logger.info("ReadyEvent!");
});

If you want to use both the gateway (to enable intents) and the event, you should use the method 1. I provided early.

[Azn9]

Do you think I got lucky or there is some behaviour change behind the hood ?

This is 100% a behaviour change introduced by the .gateway() method. We should add a warning in the docs and examples for the newcomers 😅

[frayien]

Ohhh I see, I did not notice the repro code I used for 3.1.8 did not have .gateway() ...

This is even weirder then 🤔
The more I think about it the more confused I get :

• Why does it work without race condition at all ?
• What does it change being the first event subscribed to ? Why does it matter ? This is weirdly inconsistent ...
• Would this work without fail too ?

client = DiscordClientBuilder.create(config.discord_token).build();
gateway = client.login().block();

Thread.sleep(100000);

gateway.getEventDispatcher().on(ConnectEvent.class).subscribe(event -> { System.out.println("Connect called"); });
gateway.getEventDispatcher().on(VoiceStateUpdateEvent.class).subscribe(event -> { System.out.println("Voice state called"); });

[frayien]

Thread.sleep(100000);

Hilariously enough

client = DiscordClientBuilder.create(config.discord_token).build();
gateway = client.gateway().setEnabledIntents(IntentSet.all()).login().block();

Thread.sleep(100000);

gateway.getEventDispatcher().on(ConnectEvent.class).subscribe(event -> { System.out.println("Connect called"); });
gateway.getEventDispatcher().on(VoiceStateUpdateEvent.class).subscribe(event -> { System.out.println("Voice state called"); });

Works perfectly on 3.2.5 so we can rule out race condition I guess.

[Azn9]

I found something that might explain everything, but I'm not sure that's really the reason

[Azn9]

And in your case, as your "first subscription" is not listening for "startup events", they will be dismissed

[Azn9]

Yup! That's it!

By changing the default EventDispatcher to one that replays the events, you can register any event without order:

var client = DiscordClientBuilder.create("<token>")
        .build()
        .gateway()
        .setEventDispatcher(EventDispatcher.replaying()) // <------
        .setEnabledIntents(IntentSet.all())
        .login()
        .block();

client.on(MessageCreateEvent.class).subscribe(readyEvent -> {
    logger.info("MessageCreateEvent!");
});

client.on(ConnectEvent.class).subscribe(readyEvent -> {
    logger.info("ConnectEvent!1");
});

And if you wander what does the "replay" does:

So in fact this is not an issue but an intended (but weird, I agree) behaviour

[Azn9]

(And that also explains why adding a Thread.sleep make the event still appears)

[frayien]

And in your case, as your "first subscription" is not listening for "startup events", they will be dismissed

Seems like it.

However

client = DiscordClientBuilder.create(config.discord_token).build();
gateway = client.login().block();

Thread.sleep(100000);

gateway.getEventDispatcher().on(VoiceStateUpdateEvent.class).subscribe(event -> { System.out.println("Voice state called"); });
gateway.getEventDispatcher().on(ConnectEvent.class).subscribe(event -> { System.out.println("Connect called"); });

(i.e. without using .gateway())

Works on 2.1.8 but not on 2.2.5.
Maybe it is not just about .gateway() but also event buffering changed ?

[Azn9]

Yup, that's intended, the default eventDispatcher only dispatches startup events to the first subscription made. In your case that's a listen for "VoiceStateUpdateEvent", so all the startup event saved will be dropped

[Azn9]

To adress this issue, you have three options then:

1. Use the 1. from my first answer
2. Replace event dispatcher (with .gateway().setEventDispatcher(EventDispatcher.replaying()))
3. Place the ConnectEvent listener first

[frayien]

By changing the default EventDispatcher to one that replays the events, you can register any event without order:

Yep .setEventDispatcher(EventDispatcher.replaying()) works !

Yup, that's intended

Weird then, it means the default event dispatcher got changed without obvious reasons and without being documented in the migration guide ? (Tho I guess this may be too specific to make it to the migration guide)

To adress this issue, you have three options then:

Yup, good to have found the root issue, the .setEventDispatcher(EventDispatcher.replaying()) solution may not be the most recommended one but it's good it exists :) .

Would be interesting to dig why the default even dispatcher got changed in the first place tho. Would be good to know if it is a fluke or if there is an underlying issue with replaying...

[Azn9]

By "intended" I mean that the documentation (javadoc) correspond to the behaviour we experience, there is no bug here.

👋 @quanticc if you have any idea why this changed, you know better than me 😅 (I ping you specifically bc you seems to be more active than the other core team members, sorry if I shouldn't have)

[frayien]

Found it :

private EventDispatcher initEventDispatcher(ReactorResources reactorResources) {
    if (eventDispatcher != null) {
        return eventDispatcher;
    }
    return ReplayingEventDispatcher.builder()
            .timedTaskScheduler(reactorResources.getTimerTaskScheduler())
            .build();
}

In discord4j.core.shard.GatewayBootstrap.java line 990 in 3.1.x
got changed to

private EventDispatcher initEventDispatcher() {
    if (eventDispatcher != null) {
        return eventDispatcher;
    }
    return EventDispatcher.buffering();
}

In discord4j.core.shard.GatewayBootstrap.java line 902 in 3.2.x

[frayien]

Seems kinda weird
As of October 27 2019 it was replaying or bufferer according to the boolean awaitAllShards (earliest I could go)

Then as of December 25 2019 it got changed to buffered
b7c295e

As of June 12 2020, buffered got changed to replaying in 2.1.x (current 2.1.8)
8999673

Then on November 22 2020 it got reverted to buffered but only in 2.2.x (current 2.2.5)
c23dff8

(all by quanticc so I guess you pinged the right person :) )

[quanticc]

Thanks for bringing this up, from what I can remember to explain the weirdness:

In Discord4J 3.0, sharding/login process was done differently:

• You had to attach all event listeners before calling login, that completed on logout
• Sharding used a separate class

This meant you could register all listeners before a websocket is opened using DiscordClient.getEventDispatcher().

Since our event dispatchers use EmitterProcessor under the hood:

• an EmitterProcessor buffers all events until the first subscriber
• later subscribers only get new events
• but because all subscribers are up before the first event is published, it works

However, when we introduced Discord4J 3.1, login started functioning as a "completed on gateway handshake" Mono due to multiple reasons, for instance:

• Built-in sharding, multiple shards send events to a single EventDispatcher / Store
• Split DiscordClient (REST only) from N gateway connections (GatewayDiscordClient)

Registering listeners using on after login, changed from what was expected if you listened to early events. This is missing from v3.1 migration notes.

To keep supporting old 2.x/3.0 behavior as much as possible, client.gateway().withEventDispatcher(...) was introduced, while trying to build some "smart replaying" dispatcher (ReplayingEventDispatcher) that could:

• Replay early events so multiple on calls could see known startup events (using a filter)
• Avoid saving everything due to memory concerns and also because it breaks retry or repeat calls should you use those
• After that replay window, work as a normal Emitter processor
• Revert to replay mode if all subscribers disconnect to avoid buffering everything

The result depended on API that Reactor announced it will deprecate in a future release: there's less incentive to support ReplayingEventDispatcher going forward.

Therefore it didn't become the default option in v3.2, because withEventDispatcher solves early event use cases with a simpler implementation. This is missing from migration notes

We also introduced .on(ReactiveEventAdapter) that does 1 subscription to the dispatcher, meaning it can work for early events too. For the time being, stick to one of these two methods if your bot needs early events.

Definitely an issue on our end lacking docs about event listening methods, I hope for now this can clear the confusion.