feat: implementing request timeouts in the node sdk (#182)

* feat: first pass on fetch timeouts

* test: removing timeout tests for now

* chore: docs

* fix: timeout is no longer user-configurable

Author: erunion

packages/node/README.md

@@ -57,11 +57,11 @@ app.use(readme.metrics('', req => ({
 
 | Option | Use |
 | :--- | :--- |
-| development | **default: false** If true, the log will be separate from normal production logs. This is great for separating staging or test data from data coming from customers |
-| bufferLength | **default: 10** By default, we only send logs to ReadMe after 10 requests are made. Depending on the usage of your API it make make sense to send logs more or less frequently |
+| development | **default: false** If true, the log will be separate from normal production logs. This is great for separating staging or test data from data coming from customers. |
+| bufferLength | **default: 10** By default, we only send logs to ReadMe after 10 requests are made. Depending on the usage of your API it make make sense to send logs more or less frequently. |
 | blacklist | **optional** An array of keys from your API requests and responses headers and bodies that you wish to blacklist from sending to ReadMe.<br /><br />If you configure a blacklist, it will override any whitelist configuration. |
 | whitelist | **optional** An array of keys from your API requests and responses headers and bodies that you only wish to send to ReadMe. |
-| baseLogUrl | **optional** This is the base URL for your ReadMe project. Normally this would be `https://projectName.readme.io` or `https://docs.yourdomain.com`, however if this value is not supplied, a request to the ReadMe API will be made once a day to retrieve it. This data is cached into `node_modules/.cache/readmeio`.
+| baseLogUrl | **optional** This is the base URL for your ReadMe project. Normally this would be `https://projectName.readme.io` or `https://docs.yourdomain.com`, however if this value is not supplied, a request to the ReadMe API will be made once a day to retrieve it. This data is cached into `node_modules/.cache/readmeio`. |
 
 ### Limitations
 - Currently only supports JSON request bodies. Adding a whitelist/blacklist for non-JSON bodies will not work (unless they're added to `req.body`) the same way that `body-parser` does it. The properties will be passed into [`postData`](http://www.softwareishard.com/blog/har-12-spec/#postData) as a `params` array.

packages/node/__tests__/index.test.js

@@ -160,6 +160,12 @@ describe('#metrics', () => {
       });
   });
 
+  describe('#timeout', () => {
+    it.todo('should silently fail metrics requests if they take longer than the timeout');
+
+    it.todo('should silently fail baseLogUrl requests if they take longer than the timeout');
+  });
+
   describe('#bufferLength', () => {
     it('should send requests when number hits `bufferLength` size', async function test() {
       const apiMock = getReadMeApiMock(1);

packages/node/config/default.json

@@ -1,5 +1,6 @@
 {
   "host": "https://metrics.readme.io",
   "readmeApiUrl": "https://dash.readme.io/api",
-  "bufferLength": 1
+  "bufferLength": 1,
+  "timeout": 2000
 }

packages/node/index.js

@@ -1,4 +1,5 @@
 const fetch = require('node-fetch');
+const timeoutSignal = require('timeout-signal');
 const { v4: uuidv4 } = require('uuid');
 const crypto = require('crypto');
 const config = require('./config');
@@ -31,7 +32,7 @@ function patchResponse(res) {
   };
 }
 
-async function getProjectBaseUrl(encodedApiKey) {
+async function getProjectBaseUrl(encodedApiKey, requestTimeout) {
   const cacheDir = findCacheDir({ name: pkg.name, create: true });
   const fsSafeApikey = crypto.createHash('md5').update(encodedApiKey).digest('hex');
 
@@ -49,13 +50,16 @@ async function getProjectBaseUrl(encodedApiKey) {
     lastUpdated === undefined ||
     (lastUpdated !== undefined && Math.abs(lastUpdated - Math.round(Date.now() / 1000)) >= 86400)
   ) {
+    const signal = timeoutSignal(requestTimeout);
+
     let baseUrl;
     await fetch(`${config.readmeApiUrl}/v1/`, {
       method: 'get',
       headers: {
         Authorization: `Basic ${encodedApiKey}`,
         'User-Agent': `${pkg.name}/${pkg.version}`,
       },
+      signal,
     })
       .then(res => {
         if (res.status >= 400 && res.status <= 599) {
@@ -75,6 +79,9 @@ async function getProjectBaseUrl(encodedApiKey) {
         // now yesterday so that in 2 minutes we'll automatically make another attempt.
         cache.setKey('baseUrl', null);
         cache.setKey('lastUpdated', Math.round(Date.now() / 1000) - 86400 + 120);
+      })
+      .finally(() => {
+        timeoutSignal.clear(signal);
       });
 
     cache.save();
@@ -90,13 +97,14 @@ module.exports.metrics = (apiKey, group, options = {}) => {
   if (!group) throw new Error('You must provide a grouping function');
 
   const bufferLength = options.bufferLength || config.bufferLength;
+  const requestTimeout = config.timeout;
   const encodedApiKey = Buffer.from(`${apiKey}:`).toString('base64');
   let baseLogUrl = options.baseLogUrl || undefined;
   let queue = [];
 
   return async (req, res, next) => {
     if (baseLogUrl === undefined) {
-      baseLogUrl = await getProjectBaseUrl(encodedApiKey);
+      baseLogUrl = await getProjectBaseUrl(encodedApiKey, requestTimeout);
     }
 
     const startedDateTime = new Date();
@@ -117,6 +125,9 @@ module.exports.metrics = (apiKey, group, options = {}) => {
       if (queue.length >= bufferLength) {
         const json = queue.slice();
         queue = [];
+
+        const signal = timeoutSignal(requestTimeout);
+
         fetch(`${config.host}/v1/request`, {
           method: 'post',
           body: JSON.stringify(json),
@@ -125,9 +136,15 @@ module.exports.metrics = (apiKey, group, options = {}) => {
             'Content-Type': 'application/json',
             'User-Agent': `${pkg.name}/${pkg.version}`,
           },
+          signal,
         })
           .then(() => {})
-          .catch(() => {});
+          .catch(() => {
+            // Silently discard errors and timeouts.
+          })
+          .finally(() => {
+            timeoutSignal.clear(signal);
+          });
       }
 
       cleanup(); // eslint-disable-line no-use-before-define

packages/node/package-lock.json

@@ -25,6 +25,7 @@
     "flat-cache": "^3.0.4",
     "lodash": "^4.17.15",
     "node-fetch": "^2.6.0",
+    "timeout-signal": "^1.1.0",
     "uuid": "^8.2.0"
   },
   "devDependencies": {