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: #15539 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
nodejs · Sep 25, 2017 · 9fc8edd · 9fc8edd
1 parent 1691827
commit 9fc8edd
Show file tree

Hide file tree

Showing 6 changed files with 32 additions and 20 deletions.
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -205,6 +205,7 @@ E('ERR_INVALID_FD', (fd) => `"fd" must be a positive integer: ${fd}`);
 E('ERR_INVALID_FILE_URL_HOST', 'File URL host %s');
 E('ERR_INVALID_FILE_URL_PATH', 'File URL path %s');
 E('ERR_INVALID_HANDLE_TYPE', 'This handle type cannot be sent');
+E('ERR_INVALID_HTTP_TOKEN', '%s must be a valid HTTP token ["%s"]');
 E('ERR_INVALID_OPT_VALUE',
   (name, value) => {
     return `The value "${String(value)}" is invalid for option "${name}"`;

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-compat-serverresponse-createpushresponse.js b/test/parallel/test-http2-compat-serverresponse-createpushresponse.js
@@ -25,7 +25,7 @@ const server = h2.createServer((request, response) => {
     {
       code: 'ERR_INVALID_CALLBACK',
       type: TypeError,
-      message: 'Callback must be a function'
+      message: 'callback must be a function'
     }
   );
 

diff --git a/test/parallel/test-http2-compat-serverresponse-trailers.js b/test/parallel/test-http2-compat-serverresponse-trailers.js
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);