Request timeout

doc/provider.markdown

@@ -101,7 +101,7 @@ If you wish to send notifications containing emoji or other multi-byte character
 
 Indicate to node-apn that it should close all open connections when the queue of pending notifications is fully drained. This will allow your application to terminate.
 
-**Note:** If notifications are pushed after the connection has completely shutdown a new connection will be established. However, the shutdown flag will remain and after the notifications are sent the connections will be optimistically shutdown again. Do not rely on this behaviour, it's more of a quirk.
+**Note:** If notifications are pushed after the connection has started, an error will be thrown.
 
 [provider-api]: https://developer.apple.com/library/content/documentation/NetworkingInternet/Conceptual/RemoteNotificationsPG/CommunicatingwithAPNs.html
 [provider-auth-tokens]: https://developer.apple.com/library/content/documentation/NetworkingInternet/Conceptual/RemoteNotificationsPG/CommunicatingwithAPNs.html#//apple_ref/doc/uid/TP40008194-CH11-SW1

lib/client.js

@@ -14,64 +14,104 @@ module.exports = function (dependencies) {
     HTTP2_HEADER_METHOD,
     HTTP2_HEADER_AUTHORITY,
     HTTP2_HEADER_PATH,
-    HTTP2_METHOD_POST
+    HTTP2_METHOD_POST,
+    NGHTTP2_CANCEL,
   } = http2.constants;
 
   function Client (options) {
     this.config = config(options);
     this.healthCheckInterval = setInterval(() => {
-         if (this.session && !this.session.destroyed) {
-             this.session.ping((error, duration) => {
-                 if (error) {
-                     logger("No Ping response after " + duration + " ms");
-                 }
-                 logger("Ping response after " + duration + " ms");
-             })
-         }
-     }, this.config.heartBeat).unref();
+      if (this.session && !this.session.closed && !this.session.destroyed && !this.isDestroyed) {
+        this.session.ping((error, duration) => {
+          if (error) {
+            logger("No Ping response after " + duration + " ms with error:" + error.message);
+            return;
+          }
+          logger("Ping response after " + duration + " ms");
+        });
+      }
+    }, this.config.heartBeat).unref();
+  }
+
+  // Session should be passed except when destroying the client
+  Client.prototype.destroySession = function (session, callback) {
+    if (!session) {
+      session = this.session;
+    }
+    if (session) {
+      if (this.session === session) {
+        this.session = null;
+      }
+      if (!session.destroyed) {
+        session.destroy();
+      }
+    }
+    if (callback) {
+      callback();
+    }
+  }
+
+  // Session should be passed except when destroying the client
+  Client.prototype.closeAndDestroySession = function (session, callback) {
+    if (!session) {
+      session = this.session;
+    }
+    if (session) {
+      if (this.session === session) {
+        this.session = null;
+      }
+      if (!session.closed) {
+        session.close(() => this.destroySession(session, callback));
+      } else {
+        this.destroySession(session, callback)
+      }
+    } else if (callback) {
+      callback();
+    }
   }
 
   Client.prototype.write = function write (notification, device, count) {
+    if (this.isDestroyed) {
+      return Promise.resolve({ device, error: new VError("client is destroyed") });
+    }
+
     // Connect session
-    if (!this.session || this.session.destroyed) {
-      this.session = http2.connect(`https://${this.config.address}`, this.config);
+    if (!this.session || this.session.closed || this.session.destroyed) {
+      const session = this.session = http2.connect(this._mockOverrideUrl || `https://${this.config.address}`, this.config);
+
+      this.session.on("close", () => {
+        if (logger.enabled) {
+          logger("Session closed");
+        }
+        this.destroySession(session);
+      });
 
       this.session.on("socketError", (error) => {
         if (logger.enabled) {
           logger(`Socket error: ${error}`);
         }
-        if (this.session && !this.session.destroyed) {
-          this.session.destroy();
-        }
+        this.closeAndDestroySession(session);
       });
+
       this.session.on("error", (error) => {
         if (logger.enabled) {
           logger(`Session error: ${error}`);
         }
-        if (this.session && !this.session.destroyed) {
-          this.session.destroy();
-        }
+        this.closeAndDestroySession(session);
       });
 
       this.session.on("goaway", (errorCode, lastStreamId, opaqueData) => {
-         logger(`GOAWAY received: (errorCode ${errorCode}, lastStreamId: ${lastStreamId}, opaqueData: ${opaqueData})`);
-         // gracefully stop accepting new streams
-         const session = this.session;
-         this.session = undefined;
-         if (session && !session.destroyed) {
-           session.close(() => {
-             session.destroy();
-           });
-         }
-       });
+        if (logger.enabled) {
+          logger(`GOAWAY received: (errorCode ${errorCode}, lastStreamId: ${lastStreamId}, opaqueData: ${opaqueData})`);
+        }
+        this.closeAndDestroySession(session);
+      });
 
       if (logger.enabled) {
         this.session.on("connect", () => {
           logger("Session connected");
         });
-        this.session.on("close", () => {
-          logger("Session closed");
-        });
+
         this.session.on("frameError", (frameType, errorCode, streamId) => {
           logger(`Frame error: (frameType: ${frameType}, errorCode ${errorCode}, streamId: ${streamId})`);
         });
@@ -114,62 +154,98 @@ module.exports = function (dependencies) {
 
     return new Promise ( resolve => {
       request.on("end", () => {
+        try {
+          if (logger.enabled) {
+            logger(`Request ended with status ${status} and responseData: ${responseData}`);
+          }
+
+          if (status === 200) {
+            resolve({ device });
+          } else if (responseData !== "") {
+            const response = JSON.parse(responseData);
+
+            if (status === 403 && response.reason === "ExpiredProviderToken" && retryCount < 2) {
+              this.config.token.regenerate(tokenGeneration);
+              resolve(this.write(notification, device, retryCount + 1));
+              return;
+            } else if (status === 500 && response.reason === "InternalServerError") {
+              this.closeAndDestroySession();
+              let error = new VError("Error 500, stream ended unexpectedly");
+              resolve({ device, error });
+              return;
+            }
+
+            resolve({ device, status, response });
+          } else {
+            let error = new VError("stream ended unexpectedly");
+            resolve({ device, error });
+          }
+        } catch (e) {
+          const error = new VError(e, 'Unexpected error processing APNs response');
+          if (logger.enabled) {
+            logger(`Unexpected error processing APNs response: ${e.message}`);
+          }
+          resolve({ device, error });
+        }
+      });
+
+      request.setTimeout(this.config.requestTimeout, () => {
         if (logger.enabled) {
-          logger(`Request ended with status ${status} and responseData: ${responseData}`);
+          logger('Request timeout');
         }
 
-        if (status === 200) {
-          resolve({ device });
-        } else if (responseData !== "") {
-          const response = JSON.parse(responseData);
+        request.close(NGHTTP2_CANCEL);
 
-          if (status === 403 && response.reason === "ExpiredProviderToken" && retryCount < 2) {
-            this.config.token.regenerate(tokenGeneration);
-            resolve(this.write(notification, device, retryCount + 1));
-            return;
-          } else if (status === 500 && response.reason === "InternalServerError") {
-            this.session.destroy();
-            let error = new VError("Error 500, stream ended unexpectedly");
-            resolve({ device, error });
-            return;
-          }
+        resolve({ device, error: new VError("apn write timeout") });
+      });
 
-          resolve({ device, status, response });
-        } else {
-          let error = new VError("stream ended unexpectedly");
-          resolve({ device, error });
+      request.on("aborted", () => {
+        if (logger.enabled) {
+          logger('Request aborted');
         }
-      })
+
+        resolve({ device, error: new VError("apn write aborted") });
+      });
 
       request.on("error", (error) => {
         if (logger.enabled) {
           logger(`Request error: ${error}`);
         }
 
         if (typeof error === "string") {
-          error = new VError("apn write failed: %s", err);
+          error = new VError("apn write failed: %s", error);
         } else {
           error = new VError(error, "apn write failed");
         }
         resolve({ device, error });
       });
 
+      if (logger.enabled) {
+        request.on("frameError", (frameType, errorCode, streamId) => {
+          logger(`Request frame error: (frameType: ${frameType}, errorCode ${errorCode}, streamId: ${streamId})`);
+        });
+      }
+
       request.end();
     });
   };
 
   Client.prototype.shutdown = function shutdown(callback) {
+    if (this.isDestroyed) {
+      if (callback) {
+        callback();
+      }
+      return;
+    }
+    if (logger.enabled) {
+      logger('Called client.shutdown()');
+    }
+    this.isDestroyed = true;
     if (this.healthCheckInterval) {
       clearInterval(this.healthCheckInterval);
+      this.healthCheckInterval = null;
     }
-    if (this.session && !this.session.destroyed) {
-      this.session.close(() => {
-        this.session.destroy();
-        if (callback) {
-          callback();
-        }
-      });
-    }
+    this.closeAndDestroySession(undefined, callback);
   };
 
   return Client;

lib/config.js

@@ -28,6 +28,7 @@ module.exports = function(dependencies) {
       rejectUnauthorized: true,
       connectionRetryLimit: 10,
       heartBeat: 60000,
+      requestTimeout: 5000,
     };
 
     validateOptions(options);