apn - npm Package Compare versions

Comparing version 1.6.0 to 1.6.1

ChangeLog.md

 ## Changelog
 
+1.6.1:
+
+* Increased default cache length to 1000; Previous value of 100 was overly cautious.
+* Added a delay of 100ms to connection to ensure notifications aren't lost if wrong certificates are used.
+* Fixes #195: Better handling of socket creation
+* Fixes #200: Improved buffer handling code when multiple connections are enabled.
+* Minor optimisation to Notification processing, removing the need for 2 `JSON.stringify` calls
+* Fixes #196: Check cache has contents before trying to access. Also ensure minimum size is "1" to allow "transmitted" events to be emitted.
+* Fixes #199: Emit a "drain" event when no further notifications require sending. Most useful in a batch environment.
+
 1.6.0:
@@ -4,0 +14,0 @@

lib/connection.js

@@ -13,5 +13,7 @@ var Errors = require('./errors');
 var debug = function() {};
+var trace = function() {};
 if(process.env.DEBUG) {
 	try {
 		debug = require('debug')('apn');
+		trace = require('debug')('apn:trace');
 	}
@@ -38,3 +40,3 @@ catch (e) {
  * @config {Function} [errorCallback] A callback which accepts 2 parameters (err, notification). Use `transmissionError` event instead.
- * @config {Number} [cacheLength=100] Number of notifications to cache for error purposes (See doc/apn.markdown)
+ * @config {Number} [cacheLength=1000] Number of notifications to cache for error purposes (See doc/apn.markdown)
  * @config {Boolean} [autoAdjustCache=false] Whether the cache should grow in response to messages being lost after errors. (Will still emit a 'cacheTooSmall' event)
@@ -62,5 +64,6 @@ * @config {Number} [maxConnections=1] The maximum number of connections to create for sending messages.
 		enhanced: true,
-		cacheLength: 100,
+		cacheLength: 1000,
 		autoAdjustCache: true,
 		maxConnections: 1,
+		connectTimeout: 10000,
 		connectionTimeout: 0,
@@ -107,2 +110,6 @@ connectionRetryLimit: 10,
 
+	// Set cache length to 1 to ensure transmitted notifications can be sent.
+	this.options.cacheLength = Math.max(this.options.cacheLength, 1);
+	this.options.maxConnections = Math.max(this.options.maxConnections, 1);
+
 	this.initializationPromise = null;
@@ -117,2 +124,3 @@ this.deferredConnection = null;
 	this.failureCount = 0;
+	this.connectionTimer = null;
 
@@ -122,2 +130,6 @@ // when true, we end all sockets after the pending notifications reach 0
 
+	// track when notifications are queued so transmitCompleted is only emitted one when 
+	// notifications are transmitted rather than after socket timeouts
+	this.notificationsQueued = false;
+
 	this.terminated = false;
@@ -176,4 +188,20 @@
 				this.deferredConnection.resolve();
+				
+				clearTimeout(this.connectionTimer);
+				this.connectionTimer = null;
 			}.bind(this));
 
+		this.socket.on("error",   this.errorOccurred.bind(this, this.socket));
+		this.socket.on("timeout", this.socketTimeout.bind(this, this.socket));
+		this.socket.on("data",    this.handleTransmissionError.bind(this, this.socket));
+		this.socket.on("drain",   this.socketDrained.bind(this, this.socket, true));
+		this.socket.once("close", this.socketClosed.bind(this, this.socket));
+
+		if (this.options.connectTimeout > 0) {
+			this.connectionTimer = setTimeout(function () {
+				this.deferredConnection.reject(new Error("Connect timed out"));
+				this.socket.end();
+			}.bind(this), this.options.connectTimeout);
+		}
+
 	}.bind(this)).fail(function (error) {
@@ -195,10 +223,22 @@ debug("Module initialisation error:", error);
 Connection.prototype.createConnection = function() {
-	this.connect().then(function () {
+	if (this.initialisingConnection() || this.sockets.length >= this.options.maxConnections) {
+		return;
+	}
+
+	// Delay here because Apple will successfully authenticate production certificates
+	// in sandbox, but will then immediately close the connection. Necessary to wait for a beat
+	// to see if the connection stays before sending anything because the connection will be closed
+	// without error and messages could be lost.
+	this.connect().delay(100).then(function () {
+		if (this.socket.apnRetired) {
+			throw new Error("Socket unusable after connection. Hint: You may be using a certificate for the wrong environment");
+		}
 		this.failureCount = 0;
 
-		this.socket.socketId = this.socketId++;
-		this.socket.currentId = 0;
-		this.socket.cachedNotifications = [];
+		this.socket.apnSocketId = this.socketId++;
+		this.socket.apnCurrentId = 0;
+		this.socket.apnCachedNotifications = [];
 
 		this.sockets.push(this.socket);
+		trace("connection established", this.socketId);
 	}.bind(this)).fail(function (error) {
@@ -208,2 +248,4 @@ // Exponential backoff when connections fail.
 
+		trace("connection failed", delay);
+
 		this.raiseError(error);
@@ -223,2 +265,3 @@ this.emit('error', error);
 	}.bind(this)).finally(function () {
+		trace("create completed", this.sockets.length);
 		this.deferredConnection = null;
@@ -247,2 +290,3 @@ this.socket = undefined;
 	var repeat = false;
+	var socketsAvailable = 0;
 	if(this.options.fastMode) {
@@ -252,40 +296,49 @@ repeat = true;
 	do {
-		socket = null;
-		if (this.notificationBuffer.length === 0) break;
-		for (var i = this.sockets.length - 1; i >= 0; i--) {
-			if(this.socketAvailable(this.sockets[i])) {
-				socket = this.sockets[i];
-				break;
+		socketsAvailable = 0;
+		for (var i = 0; i < this.sockets.length; i++) {
+			socket = this.sockets[i];
+			if(!this.socketAvailable(socket)) {
+				continue;
 			}
-		}
-		if (socket !== null) {
-			debug("Transmitting notification from buffer");
-			if(this.transmitNotification(socket, this.notificationBuffer.shift())) {
+
+			if (this.notificationBuffer.length === 0) {
+				socketsAvailable += 1;
+				continue;
+			}
+
+			// If a socket is available then transmit. If true is returned then manually call socketDrained
+			if (this.transmitNotification(socket, this.notificationBuffer.shift())) {
+				// Only set socket available here because if transmit returns false then the socket
+				//  is blocked so shouldn't be used in the next loop.
+				socketsAvailable += 1;
 				this.socketDrained(socket, !repeat);
 			}
 		}
-		else if (!this.initialisingConnection() && this.sockets.length < this.options.maxConnections) {
-			this.createConnection();
-			repeat = false;
+	} while(repeat && socketsAvailable > 0 && this.notificationBuffer.length > 0);
+
+	if (this.notificationBuffer.length > 0 && socketsAvailable == 0) {
+		this.createConnection();
+	}
+	
+	if (this.notificationBuffer.length === 0 && socketsAvailable == this.sockets.length){
+		if (this.notificationsQueued) {
+			this.emit('completed');
+			this.notificationsQueued = false;
 		}
-		else {
-			repeat = false;
-		}
-	} while(repeat);
-	debug("%d left to send", this.notificationBuffer.length);
+		if (this.shutdownPending) {
+			debug("closing connections");
 
-	if (this.notificationBuffer.length === 0 && this.shutdownPending) {
-		debug("closing connections");
-
-		for (var i = this.sockets.length - 1; i >= 0; i--) {
-			var socket = this.sockets[i];
-			if (!socket.busy) {
+			for (var i = 0; i < this.sockets.length; i++) {
+				var socket = this.sockets[i];
 				// We delay before closing connections to ensure we don't miss any error packets from the service.
 				setTimeout(socket.end.bind(socket), 2500);
+				this.retireSocket(socket);
 			}
+			if (this.sockets.length == 0) {
+				this.shutdownPending = false;
+			}
 		}
-		if (this.sockets.length == 0) {
-			this.shutdownPending = false;
-		}
 	}
+
+	debug("%d left to send", this.notificationBuffer.length);
  };
@@ -297,3 +350,3 @@
 Connection.prototype.errorOccurred = function(socket, err) {
-	debug("Socket error occurred", socket.socketId, err);
+	debug("Socket error occurred", socket.apnSocketId, err);
 
@@ -313,5 +366,5 @@ if(socket.transmissionErrorOccurred && err.code == 'EPIPE') {
 
-	if(socket.busy && socket.cachedNotifications.length > 0) {
+	if(socket.apnBusy && socket.apnCachedNotifications.length > 0) {
 		// A notification was in flight. It should be buffered for resending.
-		this.bufferNotification(socket.cachedNotifications[socket.cachedNotifications.length - 1]);
+		this.bufferNotification(socket.apnCachedNotifications[socket.apnCachedNotifications.length - 1]);
 	}
@@ -326,3 +379,3 @@
 Connection.prototype.socketAvailable = function(socket) {
-	if (!socket || !socket.writable || socket.busy || socket.transmissionErrorOccurred) {
+	if (!socket || !socket.writable || socket.apnRetired || socket.apnBusy || socket.transmissionErrorOccurred) {
 		return false;
@@ -337,6 +390,6 @@ }
 Connection.prototype.socketDrained = function(socket, serviceBuffer) {
-	debug("Socket drained", socket.socketId);
-	socket.busy = false;
-	if(this.options.enhanced) {
-		var notification = socket.cachedNotifications[socket.cachedNotifications.length - 1];
+	debug("Socket drained", socket.apnSocketId);
+	socket.apnBusy = false;
+	if((!this.options.legacy || this.options.enhanced) && socket.apnCachedNotifications.length > 0) {
+		var notification = socket.apnCachedNotifications[socket.apnCachedNotifications.length - 1];
 		this.emit('transmitted', notification.notification, notification.recipient);
@@ -361,3 +414,3 @@ }
  Connection.prototype.socketTimeout = function(socket) {
-	debug("Socket timeout", socket.socketId);
+	debug("Socket timeout", socket.apnSocketId);
 	this.emit('timeout');
@@ -373,3 +426,3 @@ this.destroyConnection(socket);
 Connection.prototype.destroyConnection = function(socket) {
-	debug("Destroying connection", socket.socketId);
+	debug("Destroying connection", socket.apnSocketId);
 	if (socket) {
@@ -385,3 +438,3 @@ this.retireSocket(socket);
 Connection.prototype.socketClosed = function(socket) {
-	debug("Socket closed", socket.socketId);
+	debug("Socket closed", socket.apnSocketId);
 
@@ -404,4 +457,5 @@ if (socket === this.socket && this.deferredConnection.promise.isPending()) {
  Connection.prototype.retireSocket = function(socket) {
- 	debug("Removing socket from pool", socket.socketId);
+ 	debug("Removing socket from pool", socket.apnSocketId);
 
+ 	socket.apnRetired = true;
  	var index = this.sockets.indexOf(socket);
@@ -431,2 +485,3 @@ if (index > -1) {
 	this.notificationBuffer.push(notification);
+	this.notificationsQueued = true;
 };
@@ -482,6 +537,6 @@
 Connection.prototype.cacheNotification = function (socket, notification) {
-	socket.cachedNotifications.push(notification);
-	if (socket.cachedNotifications.length > this.options.cacheLength) {
-		debug("Clearing notification %d from the cache", socket.cachedNotifications[0]['_uid']);
-		socket.cachedNotifications.splice(0, socket.cachedNotifications.length - this.options.cacheLength);
+	socket.apnCachedNotifications.push(notification);
+	if (socket.apnCachedNotifications.length > this.options.cacheLength) {
+		debug("Clearing notification %d from the cache", socket.apnCachedNotifications[0]['_uid']);
+		socket.apnCachedNotifications.splice(0, socket.apnCachedNotifications.length - this.options.cacheLength);
 	}
@@ -496,3 +551,3 @@ };
 		socket.transmissionErrorOccurred = true;
-		if (!this.options.enhanced) {
+		if (!this.options.enhanced && this.options.legacy) {
 			return;
@@ -509,4 +564,4 @@ }
 
-		while (socket.cachedNotifications.length) {
-			notification = socket.cachedNotifications.shift();
+		while (socket.apnCachedNotifications.length) {
+			notification = socket.apnCachedNotifications.shift();
 			if (notification['_uid'] == identifier) {
@@ -527,6 +582,6 @@ foundNotification = true;
 		else {
-			socket.cachedNotifications = temporaryCache;
+			socket.apnCachedNotifications = temporaryCache;
 
-			if(socket.cachedNotifications.length > 0) {
-				var differentialSize = socket.cachedNotifications[0]['_uid'] - identifier;
+			if(socket.apnCachedNotifications.length > 0) {
+				var differentialSize = socket.apnCachedNotifications[0]['_uid'] - identifier;
 				this.emit('cacheTooSmall', differentialSize);
@@ -542,7 +597,7 @@ if(this.options.autoAdjustCache) {
 
-		var count = socket.cachedNotifications.length;
+		var count = socket.apnCachedNotifications.length;
 		if(this.options.buffersNotifications) {
 			debug("Buffering %d notifications for resending", count);
 			for (var i = 0; i < count; ++i) {
-				notification = socket.cachedNotifications.shift();
+				notification = socket.apnCachedNotifications.shift();
 				this.bufferNotification(notification);
@@ -591,5 +646,5 @@ }
 
-	notification._uid = socket.currentId++;
-	if (socket.currentId > 0xffffffff) {
-		socket.currentId = 0;
+	notification._uid = socket.apnCurrentId++;
+	if (socket.apnCurrentId > 0xffffffff) {
+		socket.apnCurrentId = 0;
 	}
@@ -686,3 +741,3 @@ if (this.options.legacy) {
 
-	socket.busy = true;
+	socket.apnBusy = true;
 	return socket.write(data);
@@ -701,3 +756,2 @@ };
 	}
-	notification.compile();
 	return true;
@@ -704,0 +758,0 @@ };

lib/feedback.js

@@ -5,2 +5,4 @@ var CredentialLoader = require('./credentials');
 
+var createSocket = require('./socket');
+
 var q    = require('q');
@@ -124,14 +126,2 @@ var tls  = require('tls');
 
-function checkPEMType(input, type) {
-	if(input == null) {
-		return false;
-	}
-	var matches = input.match(/\-\-\-\-\-BEGIN ([A-Z\s*]+)\-\-\-\-\-/);
-
-	if (matches != null) {
-		return matches[1].indexOf(type) >= 0;
-	}
-	return false;
-}
-
 /**
@@ -151,2 +141,4 @@ * You should call {@link Feedback#start} instead of this method
 
+		socketOptions.port = this.options.port;
+		socketOptions.host = this.options.address;
 		socketOptions.pfx = pfxData;
@@ -159,6 +151,3 @@ socketOptions.cert = certData;
 
-		this.socket = tls.connect(
-			this.options['port'],
-			this.options['address'],
-			socketOptions,
+		this.socket = createSocket(this, socketOptions,
 			function () {
@@ -165,0 +154,0 @@ debug("Connection established");

lib/notification.js

@@ -287,3 +287,3 @@ /**
 Notification.prototype.length = function () {
-	return Buffer.byteLength(JSON.stringify(this), this.encoding || 'utf8');
+	return Buffer.byteLength(this.compile(), this.encoding || 'utf8');
 };
@@ -301,2 +301,3 @@
 	}
+	this.compiled = false;
 	var length;
@@ -303,0 +304,0 @@ var encoding = this.encoding || 'utf8';

lib/socket.js

 var tls = require('tls');
 var net = require('net');
 
+var debug = function() {};
+if(process.env.DEBUG) {
+	try {
+		debug = require('debug')('apn:socket');
+	} catch (e) {}
+}
+
 function destroyEPIPEFix(e) {
@@ -36,2 +43,4 @@ // When a write error occurs we miss the opportunity to
 
+	debug("connecting to: ", socketOptions['host'] + ":" + socketOptions['port']);
+
 	socketOptions.socket.setNoDelay(socketOptions.disableNagle);
@@ -43,8 +52,2 @@ socketOptions.socket.setKeepAlive(true);
 
-	socket.on("error",   connection.errorOccurred.bind(connection, socket));
-	socket.on("timeout", connection.socketTimeout.bind(connection, socket));
-	socket.on("data",    connection.handleTransmissionError.bind(connection, socket));
-	socket.on("drain",   connection.socketDrained.bind(connection, socket, true));
-	socket.once("close", connection.socketClosed.bind(connection, socket));
-
 	// The actual connection is delayed until after all the event listeners have
@@ -72,7 +75,3 @@ //  been attached.
 
-	socket.on("error",   connection.errorOccurred.bind(connection, socket));
-	socket.on("timeout", connection.socketTimeout.bind(connection, socket));
-	socket.on("data",    connection.handleTransmissionError.bind(connection, socket));
-	socket.on("drain",   connection.socketDrained.bind(connection, socket, true));
-	socket.once("close", connection.socketClosed.bind(connection, socket));
+	debug("connecting to: ", socketOptions['host'] + ":" + socketOptions['port']);
 
@@ -83,6 +82,8 @@ return socket;
 if (tls.TLSSocket) {
+	debug("Using 0.12 socket API");
 	module.exports = apnSocket;
 }
 else {
+	debug("Using legacy socket API");
 	module.exports = apnSocketLegacy;
 }

package.json

 {
   "name": "apn",
   "description": "An interface to the Apple Push Notification service for Node.js",
-  "version": "1.6.0",
+  "version": "1.6.1",
   "author": "Andrew Naylor <[e-mail]>",
@@ -6,0 +6,0 @@ "contributors": [

No alert changes

Improved metrics

Total package byte prevSize

137662

increased by2.36%

Lines of code

1881

increased by1.84%

Worsened metrics

Number of low supply chain risk alerts

21

increased by5%

No dependency changes