http2: remove unused onTimeout, add timeout tests

Remove unused onTimeout on Http2Session and Http2Stream because the correct _onTimeout is already declared and in use. Expand timeout tests to handle edge cases and additional arguments. PR-URL: nodejs/node#15539 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
addaleax · Sep 30, 2017 · ebe8c88 · ebe8c88
1 parent c458d18
commit ebe8c88
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 19 deletions.
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
@@ -2138,23 +2138,8 @@ const setTimeout = {
     return this;
   }
 };
-
-const onTimeout = {
-  configurable: false,
-  enumerable: false,
-  value: function() {
-    process.nextTick(emit.bind(this, 'timeout'));
-  }
-};
-
-Object.defineProperties(Http2Stream.prototype, {
-  setTimeout,
-  onTimeout
-});
-Object.defineProperties(Http2Session.prototype, {
-  setTimeout,
-  onTimeout
-});
+Object.defineProperty(Http2Stream.prototype, 'setTimeout', setTimeout);
+Object.defineProperty(Http2Session.prototype, 'setTimeout', setTimeout);
 
 // --------------------------------------------------------------------
 

diff --git a/test/parallel/test-http2-server-startup.js b/test/parallel/test-http2-server-startup.js
@@ -54,7 +54,7 @@ assert.doesNotThrow(() => {
     if (client)
       client.end();
   }));
-  server.setTimeout(common.platformTimeout(1000));
+  server.setTimeout(common.platformTimeout(1000), common.mustCall());
   server.listen(0, common.mustCall(() => {
     const port = server.address().port;
     client = net.connect(port, common.mustCall());
@@ -70,7 +70,7 @@ assert.doesNotThrow(() => {
     if (client)
       client.end();
   }));
-  server.setTimeout(common.platformTimeout(1000));
+  server.setTimeout(common.platformTimeout(1000), common.mustCall());
   server.listen(0, common.mustCall(() => {
     const port = server.address().port;
     client = tls.connect({

diff --git a/test/parallel/test-http2-timeouts.js b/test/parallel/test-http2-timeouts.js
@@ -14,6 +14,32 @@ server.on('stream', common.mustCall((stream) => {
     stream.respond({ ':status': 200 });
     stream.end('hello world');
   }));
+
+  // check that expected errors are thrown with wrong args
+  common.expectsError(
+    () => stream.setTimeout('100'),
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      type: TypeError,
+      message: 'The "msecs" argument must be of type number'
+    }
+  );
+  common.expectsError(
+    () => stream.setTimeout(0, Symbol('test')),
+    {
+      code: 'ERR_INVALID_CALLBACK',
+      type: TypeError,
+      message: 'Callback must be a function'
+    }
+  );
+  common.expectsError(
+    () => stream.setTimeout(100, {}),
+    {
+      code: 'ERR_INVALID_CALLBACK',
+      type: TypeError,
+      message: 'Callback must be a function'
+    }
+  );
 }));
 server.listen(0);