async pool.execute still throws error although inside try catch

[mohamedelkony]

when passing undifiend to pool.execute throws error and node crashes connection.execute works fine and execption is handled

import mysql = require('mysql2'); 
async function x1() {
let pool = await mysql.createPool({
host: 'localhost',user: 'nodejs',
password: 'nodejs', database: 'convfourierDB', 
port: 3306, debug: false })
.promise()
 try {
 await pool.execute('select * from inventory where id=?', [undefined]) 
console.log('x')
 }
 catch (error){
 console.log(error) 
} }
 x1()

when getting connection then execute works fine and execption is handled

async function x1() {
 let pool = await mysql.createPool({
 host: 'localhost', user: 'nodejs',
 password: 'nodejs', database: 'convfourierDB',
 port: 3306, debug: false })
.promise() 
let conn = await pool.getConnection() 
try { 
await conn.execute('select * from inventory where id=?', [undefined]) 
console.log('x') } 
catch (error) { 
console.log(error)
 } } 
x1()

[sidorares]

the problem is here - https://github.com/sidorares/node-mysql2/blob/dadef383098b0823a90e07fbec18639bd612adfb/lib/pool.js#L177

I think we need to change throw e; into return cb(e); but want to be careful and discuss what that could mean for backwards compatibility. Might require a major release

[barraponto]

@sidorares that's what #1359 does, right? should we add tests to that? I'm interested in fixing this.

[sidorares]

yes, I think so. I'm more interested in discussions on "what can possibly go wrong if we fix it". Should we have a major version bump or minor is enough etc

[wesgarland]

If the pool has an error handler analogous to the connection error handlers (if it doesn't, it should), you could emit an error emit and reject the promise without needing a major version bump.

[research-and-develop]

Hello guys, I just encounter this behavior. Throwing errors from callback functions are simply not catchable and I support the fix using return cb(e).
In my opinion you don't need major release for such a fix, since this error could only be caught using process.on('uncaughtException' handler or it is going to crash your app otherwise.

I don't see how it could break any backward code.

It's good that there is a workaround.

Looking forward for the fix. Cheers!

[cosminbodnariuc]

@research-and-develop how did you manage to go around this error? Could you share the workaround? I'm having the same issue using mysql2/promise. Thanks