Skip to content

Loading…

Fixed packet send callback issues #130

Merged
merged 1 commit into from

2 participants

@roamm

There were two issues here.
1. When Socket.send called with or without callback alternately, the trigger order is incorrect.
2. The 'drain' event from transport is one per packet for transports supporting framing like websocket and is all in one for those without framing like polling.

@roamm roamm Fixed packet send callback design issue
There were two issues here.
1. When Socket.send called with or without callback alternately,
   the trigger order is incorrect.
2. The 'drain' event from transport is one per packet for transports
   supporting framing like websocket and is all in one for those without
   framing like polling.
0dfa68c
@rauchg rauchg merged commit 3c968fc into socketio:master
@rauchg

@roamm this makes a test fail:

  1) server send callback should execute when message sent (polling):
     Error: expected 0 to equal 1
      at Assertion.assert (/Users/guillermorauch/Projects/engine.io/node_modules/expect.js/expect.js:99:13)
      at Assertion.be.Assertion.equal (/Users/guillermorauch/Projects/engine.io/node_modules/expect.js/expect.js:200:10)
      at Assertion.(anonymous function) [as be] (/Users/guillermorauch/Projects/engine.io/node_modules/expect.js/expect.js:73:24)
      at Object.listen.allowUpgrades [as _onTimeout] (/Users/guillermorauch/Projects/engine.io/test/server.js:795:26)
      at Timer.list.ontimeout (timers.js:101:19)
@roamm

@guille I fixed this and added 2 more test cases, in a new pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 19, 2012
  1. @roamm

    Fixed packet send callback design issue

    roamm committed
    There were two issues here.
    1. When Socket.send called with or without callback alternately,
       the trigger order is incorrect.
    2. The 'drain' event from transport is one per packet for transports
       supporting framing like websocket and is all in one for those without
       framing like polling.
Showing with 28 additions and 3 deletions.
  1. +12 −3 lib/socket.js
  2. +8 −0 lib/transports/flashsocket.js
  3. +8 −0 lib/transports/websocket.js
View
15 lib/socket.js
@@ -247,6 +247,13 @@ Socket.prototype.setupSendCallback = function () {
if ('function' == typeof seqFn) {
debug('executing send callback');
seqFn(self.transport);
+ } else if (Array.isArray(seqFn)) {
+ debug('executing batch send callback');
+ for (var i in seqFn) {
+ if ('function' == typeof seqFn) {
+ seqFn[i](self.transport);
+ }
+ }
}
}
});
@@ -288,9 +295,8 @@ Socket.prototype.sendPacket = function (type, data, callback) {
this.writeBuffer.push(packet);
//add send callback to object
- if (callback) {
- this.packetsFn.push(callback);
- }
+ this.packetsFn.push(callback);
+
this.flush();
}
};
@@ -309,6 +315,9 @@ Socket.prototype.flush = function () {
this.server.emit('flush', this, this.writeBuffer);
var wbuf = this.writeBuffer;
this.writeBuffer = [];
+ if (!this.transport.supportsFraming) {
+ this.packetsFn = [this.packetsFn];
+ }
this.transport.send(wbuf);
this.emit('drain');
this.server.emit('drain', this);
View
8 lib/transports/flashsocket.js
@@ -37,6 +37,14 @@ FlashSocket.prototype.__proto__ = WebSocket.prototype;
FlashSocket.prototype.name = 'flashsocket';
/**
+ * Advertise framing support.
+ *
+ * @api public
+ */
+
+FlashSocket.prototype.supportsFraming = true;
+
+/**
* Listens for new configuration changes of the Manager
* this way we can enable and disable the flash server.
*
View
8 lib/transports/websocket.js
@@ -56,6 +56,14 @@ WebSocket.prototype.name = 'websocket';
WebSocket.prototype.handlesUpgrades = true;
/**
+ * Advertise framing support.
+ *
+ * @api public
+ */
+
+WebSocket.prototype.supportsFraming = true;
+
+/**
* Processes the incoming data.
*
* @param {String} encoded packet
Something went wrong with that request. Please try again.