Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

isReady returns false although the client is ready #10493

Open
GeniusTimo opened this issue Sep 7, 2024 · 7 comments
Open

isReady returns false although the client is ready #10493

GeniusTimo opened this issue Sep 7, 2024 · 7 comments

Comments

@GeniusTimo
Copy link
Contributor

Which package is this bug report for?

discord.js

Issue description

I wanted to try out if my login retry strategy is working and therefore disabled a privileged gateway intent my bot is requesting on login. This makes it 100 % reproducible but of course this happens too if there are network issues or issues from Discords side on the first login attempt(s)

Steps to reproduce:

  1. Make sure the server members intent is disabled in the developer portal for the bot you're trying to log in to
  2. Start the execution of the code sample below (I used the example main file from the guide as template)
  3. Observe the first one or two failed login attempts
  4. Enable the server members intent in the developer portal again
  5. Observe the successful login attempt

I'd expect the readyClient to return true for isReady() but since PR #9942 got merged it also takes ws.destroyed into account which is still set to destroyed despite the new connection attempt is successful. So this is not a bug introduced in this PR but rather an already exisiting inconsistency with the ws.destroyed not being set back to true when the websocket has successfully connected and is fully operational I believe.

It could also be an implementation error of my retry strategy, but I assumed it's common practice to simply try again via login if it didn't work the first time (of course, with a deeper look at the returned error and not retrying infitite, but I tried to keep the code example as simple as possible)

Code sample

const { Client, Events, GatewayIntentBits } = require('discord.js');
const { token } = require('./config.json');

const client = new Client({ intents: [GatewayIntentBits.Guilds, GatewayIntentBits.GuildMembers] });

client.once(Events.ClientReady, readyClient => {
	console.log(`Ready! Logged in as ${readyClient.user.tag}`);

    console.log(`isReady(): ${readyClient.isReady()}`);
    console.log(`ws.destroyed: ${readyClient.ws.destroyed}`);
});

const loginClient = () => {
    console.log('Attempt to login the client...');

    client.login(token).catch(() => {
        console.log('Attempt to login the client failed');

        setTimeout(() => {
            loginClient();
        }, 10 * 1000);
    });
}

loginClient();

Versions

  • discord.js 14.16.1
  • Node.js 20.17.0

Issue priority

Low (slightly annoying)

Which partials do you have configured?

No Partials

Which gateway intents are you subscribing to?

Guilds, GuildMembers

I have tested this issue on a development release

No response

@didinele
Copy link
Member

didinele commented Sep 7, 2024

While an issue, I'd recommend avoiding retry logic like that. By design just about any error will be retried internally other than those "really bad" exceptions (using an intent you can't use, bad token, etc). Generally, if we throw a hard error it means just restarting won't fix it and you need to adjust something.

@GeniusTimo
Copy link
Contributor Author

My specific case was that I encountered the error "Opening handshake has timed out" and after the retry it worked by itself (at least that is my assumption so far) As I use isReady() quite often for type validation that's why everything else got messed up, unfortunately.
Is there any recommendable retry strategy at all? It sounds to me like the best idea would be to initiate a new instance of the client to make sure that everything is OK internally?

@Qjuh
Copy link
Contributor

Qjuh commented Sep 7, 2024

You are correct. As didinele already pointed out: if Client#login() rejects you shouldn't use that Client instance anymore at all, because the Client itself will be destroyed by then.

@Qjuh Qjuh removed their assignment Sep 7, 2024
@didinele
Copy link
Member

didinele commented Sep 7, 2024

My specific case was that I encountered the error "Opening handshake has timed out" and after the retry it worked by itself (at least that is my assumption so far) As I use isReady() quite often for type validation that's why everything else got messed up, unfortunately.

And it didn't recover by itself from that?

@GeniusTimo
Copy link
Contributor Author

That's something I'm not 100 % sure about. My error log stated (using discord.js 14.15.3 at that point)

Error: Opening handshake has timed out
    at ClientRequest.<anonymous> (.../node_modules/ws/lib/websocket.js:873:7)
    at ClientRequest.emit (node:events:519:28)
    at TLSSocket.emitRequestTimeout (node:_http_client:856:9)
    at Object.onceWrapper (node:events:633:28)
    at TLSSocket.emit (node:events:519:28)
    at TLSSocket.Socket._onTimeout (node:net:591:8)
    at listOnTimeout (node:internal/timers:581:17)
    at processTimers (node:internal/timers:519:7)

without a timestamp. The next log entry of my own logging told me Error: ConnectTimeoutError: Connect Timeout Error needed to be catched while attempting to log in at a timestamp that would fit the time after that my bot got unresponsive. As the error message sounds quite even I assumed that the "Opening handshake has timed out" could be the more specific error of the "ConnectTimeoutError: Connect Timeout Error". At least that's the error I'm 100 % sure that it didn't recover itself

@didinele
Copy link
Member

didinele commented Sep 7, 2024

I recommend listening to the error event (and ShardError), and also the debug event to help diagnose this sort of thing better.

@GeniusTimo
Copy link
Contributor Author

So far I was only listening to the error event in production. I added the shardError event now too. As this error did not occur in ages for me, I think it should be fine to redo my retry logic to not only reattempt the login but rather initiating a new client. That solves the problem for me, so thank you very much for your (as always) fast and helpful responses!
Possibly this issue is obsolete if the client instance is not supposed to recover from a failed login, but I don't understand why the ready event is called then and some additional documentation could be helpful to other developers implementing some sort of retry logic too!

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

No branches or pull requests

3 participants