Stop catching parser errors

felixge · felixge · commit 6d8fc34c107c · 2012-07-10T16:56:21.000-04:00
Having any exception on the parser stack trapped requires any
callback/event going back to the user to be wrapped in process.nextTick
in order for it to not be trapped by try...catch block which is very
annoying.

This also makes it hard to guarantee row streaming pause() / resume() to
work properly as invoking the 'result' events in a nextTick means that
other 'result' events may fire, even if the first one calls pause()
(because the other ones are already scheduled as nextTicks).

Even so it would be nice to allow the user to handle potentially unknown
problems in the parser gracefully, it is ultimatively not worth the
tradeoffs that are involved. Going forward any unknown parser defects
will cause hard exceptions which may only be trapped by using domains or
'uncaughtException'.

This patch is also done in preparation for implementing exception
safety, so that it becomes possible to use a shared mysql connection
across multiple domains.
diff --git a/Readme.md b/Readme.md
@@ -423,7 +423,7 @@ object. Additionally they come with two properties:
 
 * `err.code`: Either a [MySQL server error][] (e.g.
   `'ER_ACCESS_DENIED_ERROR'`), a node.js error (e.g. `'ECONNREFUSED'`) or an
-  internal error (e.g.  `'PROTOCOL_PARSER_EXCEPTION'`).
+  internal error (e.g.  `'PROTOCOL_CONNECTION_LOST'`).
 * `err.fatal`: Boolean, indicating if this error is terminal to the connection
   object.
 
diff --git a/lib/protocol/Protocol.js b/lib/protocol/Protocol.js
@@ -27,17 +27,8 @@ function Protocol(options) {
 }
 
 Protocol.prototype.write = function(buffer) {
-  try {
-    this._parser.write(buffer);
-    return true;
-  } catch (err) {
-    err.code = err.code || 'PROTOCOL_PARSER_EXCEPTION';
-    err.fatal = true;
-
-    this._delegateError(err);
-  }
-
-  return false;
+  this._parser.write(buffer);
+  return true;
 };
 
 Protocol.prototype.handshake = function(cb) {
diff --git a/lib/protocol/sequences/Sequence.js b/lib/protocol/sequences/Sequence.js
@@ -50,26 +50,17 @@ Sequence.prototype.end = function(err) {
   }
 
   this._ended = true;
-  var self    = this;
-  var args    = arguments;
 
   if (err) {
     this._addLongStackTrace(err);
+    this.emit('error', err);
   }
 
+  if (this._callback) {
+    this._callback.apply(this, arguments);
+  }
 
-  // Escape stack (so try..catch in Protocol#write does not interfer here)
-  process.nextTick(function() {
-    if (err) {
-      self.emit('error', err);
-    }
-
-    if (self._callback) {
-      self._callback.apply(self, args);
-    }
-
-    self.emit('end');
-  });
+  this.emit('end');
 };
 
 Sequence.prototype['OkPacket'] = function(packet) {
diff --git a/test/integration/test-query-after-end-without-callback.js b/test/integration/test-query-after-end-without-callback.js
@@ -11,10 +11,10 @@ connection.end(function(err) {
 });
 
 var err;
-connection.query('SELECT 1');
 connection.on('error', function(_err) {
   err = _err;
 });
+connection.query('SELECT 1');
 
 process.on('exit', function() {
   assert.equal(didEnd, true);
