Handle socket errors

[marcuswestin]

It seems like all socket errors are not being handled.

This example calling code:

mysqlClient.query(params.query)
    .on('error', function(err) {
        sendError(res, err)
    })
    .on(....)

ended up with this stack trace:

Error: Socket is not writable
    at Socket._writeOut (net.js:391:11)
    at Socket.write (net.js:377:17)
    at cmd.write (code/node_modules/mysql/lib/mysql-native/command.js:61:19)
    at cmd.start (code/node_modules/mysql/lib/mysql-native/commands/query.js:107:12)
    at cmd.process_packet (code/node_modules/mysql/lib/mysql-native/command.js:45:58)
    at SocketClient.dispatch_packet (code/node_modules/mysql/lib/mysql-native/socketclient.js:105:32)
    at SocketClient.add (code/node_modules/mysql/lib/mysql-native/socketclient.js:123:14)
    at SocketClient.query (code/node_modules/mysql/lib/mysql-native/socketclient.js:40:21)
    at Server.<anonymous> (code/graphs/scripts/run-server.js:31:15)

My best guess is that we should be checking in command.js

if (!connection.write) {
    listen for connection 'drain' event and then keep writing
}

At least the 'error' event should bubble up all the way to the client API, rather than crashing the process.

[sidorares]

The best way to start is to listen socket 'error' events.
Checking write result and listening for drain event just changes proportion of queue length/write buffer size. Did you actually encountered situation you have error because of write buffer overflow/memory limit?

[marcuswestin]

I don't know enough of the node socket API to guess what caused the error.

Listening to the error event sounds great - what should the library do on error? Republishing it to the API is a start, but I imagine the library also needs to clean up state in case there's an error.

Do you know who the right people are to ask about this? We just started using this code and I don't know who the active maintainers are.

Thanks for responding!

Cheers,
Marcus

-- while mobile

On Nov 17, 2011, at 3:08 PM, Andrey [email protected] wrote:

The best way to start is to listen socket 'error' events.
Checking write result and listening for drain event just changes proportion of queue length/write buffer size. Did you actually encountered situation you have error because of write buffer overflow/memory limit?

---

Reply to this email directly or view it on GitHub:
#51 (comment)

[sidorares]

I'm maintainer and main developer

What are the main use cases for library? I'm currently preparing big changes in API and hope to see as much feedback as possible.
What would be accepted reconnect behavior? Note that connection error could be in idle state (no commends in the queue), during initial command execution,
in the middle of command execution (ie command sent and part of result received), with or without other commands in the queue.

Please share your proposals on the ML if you think it would be useful for others.

Other than finishing 'mlc' branch development planned features are speed and more tests.

Code is on branch 'mlc'

API examples

var db = mysql.createClient(params);
db.addConnection();
db.addConnection();

// this will result in simple query
db.queue('select 1+1', function(err, res) {
console.log(res);
});

// this will result in prepare + execute
db.queue('select ?+?', [1, 2], function(err, res) {
console.log(res);
});

//next two queries are executed in parallel id there are two available connections
db.query('select sleep(5)');
db.query('select sleep(5)');

[marcuswestin]

Awesome - I love how responsive you are being! Very much appreciated.

We're using the library for an internal stats dashboard at Clover. I intend to open source it.

The minimum in the current API I believe is emitting an error event on the command/connection object and cleaning up any state associated with that command. Caller can then choose to re-issue the command, show an error, etc. The error just needs to be exposed through the API, rather than thrown.

What I like about your current API is that you're emitting a stream of row events, rather than a single JSON object. Keep it that way. Felixge's branch is already fine if you want the response in one chunk.

-- while mobile

On Nov 18, 2011, at 12:01 AM, Andrey [email protected] wrote:

I'm maintainer and main developer

What are the main use cases for library? I'm currently preparing big changes in API and hope to see as much feedback as possible.
What would be accepted reconnect behavior? Note that connection error could be in idle state (no commends in the queue), during initial command execution,
in the middle of command execution (ie command sent and part of result received), with or without other commands in the queue.

Please share your proposals on the ML if you think it would be useful for others.

Other than finishing 'mlc' branch development planned features are speed and more tests.

Code is on branch 'mlc'

API examples

var db = mysql.createClient(params);
db.addConnection();
db.addConnection();

// this will result in simple query
db.queue('select 1+1', function(err, res) {
console.log(res);
});

// this will result in prepare + execute
db.queue('select ?+?', [1, 2], function(err, res) {
console.log(res);
});

//next two queries are executed in parallel id there are two available connections
db.query('select sleep(5)');
db.query('select sleep(5)');

---

Reply to this email directly or view it on GitHub:
#51 (comment)

[marcuswestin]

I currently have this fugly hack in prod :)

function hackRestart() {
    if (mysqlClient) { mysqlClient.terminate() }
    mysqlClient = mysql.createTCPClient('bulldog.corp.clover.com')
    ...
}

...

try {
    query = mysqlClient.query(params.query)
} catch(e) {
    hackRestart()
}

[sidorares]

I hope to push connection errors & restart handling soon.
You pointed me to a much more serious problem than 'can't connect' exception: if connection is terminated (network cable on/off for example) during command execution, not only client does not notice it, but command processing freezes and if you continue adding commands you'll have eventually out of memory crash.

You can check before query if (mysqlClient.connection.writable) - this should fail if connection had not been not opened in constructor or closed since last command