await .ping() return [undefined, undefined]

[meow-developer]

I have a question in regard to the ping function. I found out that the response of the async ping function is [undefined, undefined]

async function a(){
    const b = await mysql.createConnection({
        "host": "127.0.0.1",
        "user": "admin",
        "password": "xxxxxxxx"
    });
    const c = await b.ping();
    console.log(c);
}

a();

Output: [ undefined, undefined ]

Is it that I missed something? I think I have implemented the functions correctly. The MYSQL server is running when I execute the above code.

I cannot find any documentation in this module about how can I execute the ping function asynchronously so I purely add the await in front of ping().

[sidorares]

this is because its reusing makeDoneCb helper from query/execute

node-mysql2/promise.js

Line 17 in ec4ed3e

resolve([rows, fields]);

would you like to volunteer and improve this @ansleehk?

Either make a "void" version of makeDoneCb or just have similar code inline in ping() here ( this is what .connect() is doing few lines below )

node-mysql2/promise.js

Line 165 in ec4ed3e

c.ping(done);

Also we need to check the change is covered in typings ( or correct types are added if there are none )

[meow-developer]

Working on it!

…

On Fri, 14 Oct 2022 at 16:38, Andrey Sidorov ***@***.***> wrote: this is because its reusing makeDoneCb helper from query/execute https://github.com/sidorares/node-mysql2/blob/ec4ed3ee04a94839216c50dbbb6d84efaa26b4fa/promise.js#L17 would you like to volunteer and improve this @ansleehk <https://github.com/ansleehk>? Either make a "void" version of makeDoneCb or just have similar code inline in ping() here https://github.com/sidorares/node-mysql2/blob/ec4ed3ee04a94839216c50dbbb6d84efaa26b4fa/promise.js#L165 Also we need to check the change is covered in typings ( or correct types are added if there are none ) — Reply to this email directly, view it on GitHub <#1650 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AE7MJZHYE6YSHY72WJBLXYDWDHVF7ANCNFSM6AAAAAAREWWHAI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[meow-developer]

This is not an issue purely related to the makeDoneCb function. Even though I run the following code, the three values from the callback are still "undefined".

  ping() {
    const c = this.connection;
    const localErr = new Error();
    return new this.Promise((resolve, reject) => {
        c.ping((a,b,c)=>{
          console.log(a,b,c);
        });
    });
  }

I think this issue is related to the so-called core package.
Therefore, I dig into the ping.js, which is located in lib/commands/, I think no values have been put into the callback functions, so the values of three are undefined.

[sidorares]

why do you expect any data from ping callback?

This is how its called

node-mysql2/lib/commands/ping.js

Line 30 in 33073c1

process.nextTick(this.onResult.bind(this));

This is COM_PING documentation

https://dev.mysql.com/doc/dev/mysql-server/latest/page_protocol_com_ping.html

[meow-developer]

Based on my limited understanding on the ping function, I am guessing that when an OK_Packet is sent to the client, no values will be passed into the Callback function. Yet, I am wondering whether we should pass a True to the Callback function in order to tell the server is alive and reachable? Otherwise, when a response is sent from the MySQL server within the TIMEOUT, all three values will only be undefined as ERR_PACKET will never be a return in the Response, making the ping function useless. By the way, I think I am not familiar with the ping function and how the Packet class works, do you mind taking over this issue?

[sidorares]

Currently there is no timeout you can provide to the ping() call, it'll wait until there is a response from the server ( or error because of network connection closed )

If you want to add timeout its relatively easy to do via Promise.race, example:

const timeout = (prom, time) =>
	Promise.race([prom, new Promise((_r, rej) => setTimeout(rej, time))]);
try {
  // check if server is alive but don't wait for more than 10 seconds
  await timeout(connection.ping(), 10000);
  // alive - we know that the server responded
} catch(error) {
  // not alive
}

I am guessing that when an OK_Packet is sent to the client, no values will be passed into the Callback function

yes, when OK packed is received from the server it is handled by the command expecting the response ( Ping command in our case ):

node-mysql2/lib/commands/ping.js

Line 30 in 33073c1

process.nextTick(this.onResult.bind(this));

If you confused by process.nextTick(this.onResult.bind(this)); line:
Its essentially the same as this.onResult(), but ignoring exceptions. Its meant to do the same as

// call user's callback, but if that fails don't break control here
try {
   this.onResult();
} catch(e) {
   //
}

the reason process.nextTick is used instead of try/catch is that in old versions of V8 just the fact you have try/catch anywhere in your function might cause it to deoptimize.

[meow-developer]

I really appreciate that you are telling me how the program works and another way to write the Try/catch. Yet, I am not confident in solving this issue. I have already looked into the code since I was working on this issue but I think this is too advanced for me.

[sidorares]

@ansleehk the modified ping() function should look like this:

  connect() {
    const c = this.connection;
    const localErr = new Error();
    return new this.Promise((resolve, reject) => {
      c.ping((err) => {
        if (err) {
          localErr.message = err.message;
          localErr.code = err.code;
          localErr.errno = err.errno;
          localErr.sqlState = err.sqlState;
          localErr.sqlMessage = err.sqlMessage;
          reject(localErr);
        } else {
          resolve();
        }
      });
    });
  }

The typings for ping() look already correct to me:

node-mysql2/promise.d.ts

Line 20 in 33073c1

ping(): Promise<void>;