Timeouts for HTTP Client (#150) – fixes #131

* Add HTTP_TIMEOUT setting for #131
- remove unncessary skip code
- include test recordings

* fix lint

* Added two timeout tests

* fix random test failure

src/html/fetch-markdown.js

@@ -37,7 +37,7 @@ function uri(root, owner, repo, ref, path) {
  * @param {import("../context").Action} action some other param
  */
 async function fetch(
-  { error, content: { sources = [] } = {} },
+  { content: { sources = [] } = {} },
   {
     secrets = {},
     request,
@@ -48,6 +48,13 @@ async function fetch(
     return bail(logger, 'Request parameters are missing');
   }
 
+  let timeout;
+  if (!secrets.HTTP_TIMEOUT) {
+    logger.warn('No HTTP timeout set, risk of denial-of-service');
+  } else {
+    timeout = secrets.HTTP_TIMEOUT;
+  }
+
   // get required request parameters
   const {
     owner, repo, path,
@@ -76,6 +83,7 @@ async function fetch(
   const options = {
     uri: uri(rootPath, owner, repo, ref, path),
     json: false,
+    timeout,
   };
   logger.debug(`fetching Markdown from ${options.uri}`);
   try {
@@ -87,7 +95,14 @@ async function fetch(
       },
     };
   } catch (e) {
-    return bail(logger, `Could not fetch Markdown from ${options.uri}`, e, 404);
+    if (e && e.statusCode && e.statusCode === 404) {
+      return bail(logger, `Could not find Markdown at ${options.uri}`, e, 404);
+    } if (e && e.cause && e.cause.code && (e.cause.code === 'ESOCKETTIMEDOUT' || e.cause.code === 'ETIMEDOUT')) {
+      // return gateway timeout
+      return bail(logger, `Gateway timout of ${timeout} milliseconds exceeded for ${options.uri}`, e, 504);
+    }
+    // return a bad gateway for all other errors
+    return bail(logger, `Could not fetch Markdown from ${options.uri}`, e, 502);
   }
 }
 

test/testConditionalSections.js

@@ -418,7 +418,7 @@ But neither this one.
 test: a
 ---
 
-Or this one at the same time.
+Or that one at the same time.
 
 `,
           },

test/testFetchMarkdown.js

@@ -257,3 +257,94 @@ describe('Test requests', () => {
     assert.equal(result.content.body.split('\n')[0], '# Foo Data Model (XDM) Schema');
   });
 });
+
+
+describe('Test misbehaved HTTP Responses', () => {
+  setupPolly({
+    logging: false,
+    recordFailedRequests: false,
+    adapters: [NodeHttpAdapter],
+    persister: FSPersister,
+    persisterOptions: {
+      fs: {
+        recordingsDir: 'test/fixtures',
+      },
+    },
+  });
+
+  it('Getting XDM README with bad HTTP Status Code', async function badStatus() {
+    const { server } = this.polly;
+
+    server
+      .get('https://raw.githubusercontent.com/adobe/xdm/master/README.md')
+      .intercept((_, res) => res.sendStatus(500));
+
+    const myaction = {
+      request: {
+        params: {
+          repo: 'xdm', ref: 'master', path: 'README.md', owner: 'adobe',
+        },
+      },
+      logger,
+    };
+
+    await coerce(myaction);
+
+    const result = await fetch({}, myaction);
+    assert.ok(result.error);
+    assert.equal(result.response.status, 502);
+  });
+
+  it('Getting XDM README with ultra-short Timeout', async function shortTimeout() {
+    const { server } = this.polly;
+
+    const wait = ms => new Promise(r => setTimeout(r, ms));
+
+    server
+      .get('https://raw.githubusercontent.com/adobe/xdm/master/README.md')
+      .intercept((_, res) => wait(50).then(res.sendStatus(500)));
+
+    const myaction = {
+      request: {
+        params: {
+          repo: 'xdm', ref: 'master', path: 'README.md', owner: 'adobe',
+        },
+      },
+      logger,
+      secrets: {
+        HTTP_TIMEOUT: 10,
+      },
+    };
+
+    await coerce(myaction);
+
+    const result = await fetch({}, myaction);
+    assert.ok(result.error);
+    assert.equal(result.response.status, 504);
+  });
+
+  it('Getting XDM README with Backend Timeout', async function badTimeout() {
+    const { server } = this.polly;
+
+    const wait = ms => new Promise(r => setTimeout(r, ms));
+
+    server
+      .get('https://raw.githubusercontent.com/adobe/xdm/master/README.md')
+      .intercept((_, res) => wait(2000).then(res.sendStatus(500)));
+
+    const myaction = {
+      request: {
+        params: {
+          repo: 'xdm', ref: 'master', path: 'README.md', owner: 'adobe',
+        },
+      },
+      logger,
+    };
+
+    await coerce(myaction);
+
+    const result = await fetch({}, myaction);
+    assert.ok(result.error);
+    assert.equal(result.response.status, 504);
+  }).timeout(3000);
+});