Added some tests, fixed some bugs

appveyor.yml

@@ -0,0 +1,27 @@
+# version format
+version: '{build}'
+
+# branches to build
+branches:
+  # whitelist
+  only:
+    - master
+
+# Do not build on tags (GitHub, Bitbucket, GitLab, Gitea)
+skip_tags: true
+
+# Maximum number of concurrent jobs for the project
+max_jobs: 1
+clone_depth: 1
+
+# Build worker image (VM template)
+build_cloud: Docker
+
+environment:
+  docker_image: node:16-alpine
+  npm_config_cache: /appveyor/projects/cache
+
+test_script:
+  - sh: |
+      npm install
+      npm test

core/client.mjs

@@ -3,21 +3,34 @@ import https from 'https'
 import fs from 'fs'
 import url from 'url'
 
-export function request(config, path, filePath = null, redirects, returnText = false) {
-  if (!config || typeof(config) === 'string') {
+function resolveRelative(from, to) {
+  const resolvedUrl = new URL(to, new URL(from, 'resolve://'));
+  if (resolvedUrl.protocol === 'resolve:') {
+    // `from` is a relative URL.
+    const { pathname, search, hash } = resolvedUrl;
+    return pathname + search + hash;
+  }
+  return resolvedUrl.toString();
+}
+
+export function request(config, path, filePath = null, redirects, fastRaw = false) {
+  if (!config || typeof(config) !== 'object' || Array.isArray(config)) {
     return Promise.reject(new Error('Request must be called with config in first parameter'))
   }
-  let newRedirects = redirects + 1
-  if (!path || !path.startsWith('http')) {
-    return Promise.reject(new Error('URL was empty or missing http in front'))
+  let newRedirects = (redirects || 0) + 1
+  if (!path || typeof(path) !== 'string' || !path.startsWith('http')) {
+    return Promise.reject(new Error('URL was empty or invalid'))
+  }
+  let parsed
+  try {
+    parsed = new url.URL(path)
+  } catch {
+    return Promise.reject(new Error('URL was empty or invalid'))
   }
-  let parsed = new url.URL(path)
 
-  let h
+  let h = http
   if (parsed.protocol === 'https:') {
     h = https
-  } else {
-    h = http
   }
 
   return new Promise(function(resolve, reject) {
@@ -31,14 +44,28 @@ export function request(config, path, filePath = null, redirects, returnText = f
     if (config.githubAuthToken && path.indexOf('api.github.com') >= 0) {
       headers['Authorization'] = `token ${config.githubAuthToken}`
     }
-    let req = h.request({
+    let timeout = fastRaw ? 5000 : config.timeout || 10000
+
+    let req = null
+
+    let timedout = false
+    let timer = setTimeout(function() {
+      timedout = true
+      if (req) { req.destroy() }
+      reject(new Error(`Request ${path} timed out out after ${timeout}`))
+    }, timeout)
+
+    req = h.request({
       path: parsed.pathname + parsed.search,
       port: parsed.port,
       method: 'GET',
       headers: headers,
-      timeout: returnText ? 5000 : 10000,
+      timeout: timeout,
       hostname: parsed.hostname
     }, function(res) {
+      if (timedout) { return }
+      clearTimeout(timer)
+
       let output = ''
       if (filePath) {
         let file = fs.createWriteStream(filePath)
@@ -57,9 +84,9 @@ export function request(config, path, filePath = null, redirects, returnText = f
             return reject(new Error('Redirect returned no path in location header'))
           }
           if (res.headers.location.startsWith('http')) {
-            return resolve(request(config, res.headers.location, filePath, newRedirects, returnText))
+            return resolve(request(config, res.headers.location, filePath, newRedirects, fastRaw))
           } else {
-            return resolve(request(config, url.resolve(path, res.headers.location), filePath, newRedirects, returnText))
+            return resolve(request(config, resolveRelative(path, res.headers.location), filePath, newRedirects, fastRaw))
           }
         } else if (res.statusCode >= 400) {
           return reject(new Error(`HTTP Error ${res.statusCode}: ${output}`))
@@ -72,18 +99,26 @@ export function request(config, path, filePath = null, redirects, returnText = f
           body: output
         })
       })
-      req.on('error', reject)
-      req.on('timeout', function(err) {
-        reject(err)
-      })
     })
+    
+    req.on('error', function(err) {
+      if (timedout) return
+      reject(new Error(`Error during request ${path}: ${err.message}`))
+    })
+    req.on('timeout', function(err) {
+      if (timedout) return
+      reject(err)
+    })
+
     req.end()
   }).then(function(res) {
-    if (!filePath && !returnText) {
-      try {
-        res.body = JSON.parse(res.body)
-      } catch(e) {
-        throw new Error(res.body)
+    if (!filePath && !fastRaw) {
+      if (typeof(res.body) === 'string') {
+        try {
+          res.body = JSON.parse(res.body)
+        } catch(e) {
+          throw new Error(`Error parsing body ${res.body}: ${e.message}`)
+        }
       }
     }
     return res

core/db.mjs

@@ -139,25 +139,23 @@ export default function GetDB(util, log) {
       db._.mixin(lodashId)
       db.adapterFilePath = util.getPathFromRoot('./db.json')
 
-      db.defaults({
-          core: {
-            "appActive": null, // Current active running
-            "appLatestInstalled": null, // Latest installed version
-            "appLatestVersion": null, // Newest version available
-            "manageActive": null,
-            "manageLatestInstalled": null,
-            "manageLatestVersion": null
-          },
-          core_appHistory: [],
-          core_manageHistory: [],
-          core_version: 1,
-        })
-        .write()
-        .then(
-          function() { },
-          function(e) { log.error(e, 'Error writing defaults to lowdb') }
-        )
-      
-      return db
+      return db.defaults({
+        core: {
+          "appActive": null, // Current active running
+          "appLatestInstalled": null, // Latest installed version
+          "appLatestVersion": null, // Newest version available
+          "manageActive": null,
+          "manageLatestInstalled": null,
+          "manageLatestVersion": null
+        },
+        core_appHistory: [],
+        core_manageHistory: [],
+        core_version: 1,
+      })
+      .write()
+      .then(
+        function() { db },
+        function(e) { log.error(e, 'Error writing defaults to lowdb') }
+      )
     })
 }

core/util.mjs

@@ -23,7 +23,7 @@ export default class Util {
         shell: true,
         cwd: folder,
       })
-      let timeOuter = setTimeout(function() {
+      let timeOuter = setInterval(function() {
         processor.stdin.write('n\n')
       }, 250)
       processor.stdout.on('data', function(data) {

package.json

@@ -5,7 +5,7 @@
   "main": "lib.mjs",
   "scripts": {
     "dev": "nodemon --watch dev/api --watch core --watch runner.mjs --watch db.mjs --watch log.mjs runner.mjs | bunyan",
-    "test": "echo \"Error: no test specified\" && exit 1"
+    "test": "eltro test/**/*.test.mjs -r dot"
   },
   "repository": {
     "type": "git",
@@ -29,5 +29,6 @@
     "lowdb": "^1.0.0"
   },
   "devDependencies": {
+    "eltro": "^1.3.0"
   }
 }

test/client.test.mjs

@@ -0,0 +1,164 @@
+import { Eltro as t, assert} from 'eltro'
+import http from 'http'
+import { request } from '../core/client.mjs'
+
+const port = 61412
+const defaultHandler = function(req, res) {
+  res.statusCode = 200
+  res.end('{"a":1}');
+}
+let server = null
+let prefix = `http://localhost:${port}/`
+let handler = defaultHandler
+
+t.before(function(cb) {
+  server = http.createServer(function(req, res) {
+    req.on('error', function(err) {
+      console.log('error', err)
+    })
+    res.on('error', function(err) {
+      console.log('error', err)
+    })
+    handler(req, res)
+  })
+  server.listen(port, cb)
+})
+
+t.describe('Basics', function() {
+  t.beforeEach(function() {
+    handler = defaultHandler
+  })
+
+  t.test('should require valid config', async function() {
+    function checkError(err) {
+      assert.match(err.message, /config/i)
+    }
+
+    await assert.isRejected(request(prefix)).then(checkError)
+    await assert.isRejected(request('', prefix)).then(checkError)
+    await assert.isRejected(request([], prefix)).then(checkError)
+    await assert.isRejected(request(123, prefix)).then(checkError)
+    await assert.isRejected(request(0, prefix)).then(checkError)
+  })
+
+  t.test('should fail if url is invalid', async function() {
+    function checkError(err) {
+      assert.match(err.message, /invalid/i)
+      assert.match(err.message, /url/i)
+    }
+    
+    await assert.isRejected(request({}, 123)).then(checkError)
+    await assert.isRejected(request({}, [])).then(checkError)
+    await assert.isRejected(request({}, {})).then(checkError)
+    await assert.isRejected(request({}, '')).then(checkError)
+    await assert.isRejected(request({}, 'asdf')).then(checkError)
+    await assert.isRejected(request({}, 'httpppp')).then(checkError)
+  })
+})
+
+t.describe('Request', function() {
+  t.beforeEach(function() {
+    handler = defaultHandler
+  })
+
+  t.test('should work normally', async function() {
+    let res = await request({}, prefix)
+    assert.deepEqual(res.body, {a:1})
+  })
+
+  t.test('should follow redirects', async function() {
+    let counter = 0
+    handler = function(req, res) {
+      if (counter < 3) {
+        res.statusCode = 302
+        res.setHeader('Location', encodeURI(prefix))
+        res.end();
+        counter++
+        return
+      }
+      assert.strictEqual(req.url, '/')
+      res.statusCode = 200
+      res.end('{"a":1}');
+      return
+    }
+    let res = await request({}, prefix)
+    assert.deepEqual(res.body, {a:1})
+    assert.strictEqual(counter, 3)
+  })
+
+  t.test('should fail if infinite redirect', async function() {
+    const assertRelativeLocation = 'some/text/here'
+    const assertLocation = prefix + assertRelativeLocation
+    let counter = 0
+    handler = function(req, res) {
+      res.statusCode = 302
+      res.setHeader('Location', encodeURI(assertLocation))
+      if (counter === 0) {
+        assert.strictEqual(req.url, '/')
+      } else {
+        assert.strictEqual(req.url, '/' + assertRelativeLocation)
+      }
+      res.end();
+      counter++
+    }
+
+    let err = await assert.isRejected(request({}, prefix))
+    assert.strictEqual(counter, 6)
+    assert.match(err.message, /redirect/i)
+    assert.match(err.message, new RegExp(assertRelativeLocation))
+  })
+
+  t.test('should fail if redirect is missing location', async function() {
+    let counter = 0
+    handler = function(req, res) {
+      res.statusCode = 302
+      res.end();
+      counter++
+    }
+
+    let err = await assert.isRejected(request({}, prefix))
+    assert.strictEqual(counter, 1)
+    assert.match(err.message, /redirect/i)
+    assert.match(err.message, /location/i)
+  })
+
+  t.test('should follow relative redirects', async function() {
+    const assertUrl = 'asdf1234'
+    let counter = 0
+    let url = ''
+    handler = function(req, res) {
+      if (counter < 1) {
+        res.statusCode = 302
+        res.setHeader('Location', encodeURI(assertUrl))
+        res.end();
+        counter++
+        return
+      }
+      url = req.url
+      res.statusCode = 200
+      res.end('{"a":1}');
+      return
+    }
+    let res = await request({}, prefix)
+    assert.deepEqual(res.body, {a:1})
+    assert.strictEqual(counter, 1)
+    assert.strictEqual(url, '/' + assertUrl)
+
+    counter = 0
+    res = await request({}, prefix + 'some/url/here')
+    assert.deepEqual(res.body, {a:1})
+    assert.strictEqual(counter, 1)
+    assert.strictEqual(url, '/some/url/' + assertUrl)
+  })
+
+  t.timeout(30).test('should support timeout on invalid url', async function() {
+    // blocked off port, should time out
+    let err = await assert.isRejected(request({ timeout: 15 }, 'http://git.nfp.is:8080'))
+    assert.match(err.message, /timed out/i)
+    assert.match(err.message, /15/i)
+  })
+})
+
+t.after(function(cb) {
+  server.close(cb)
+})

test/db.test.mjs

@@ -0,0 +1,2 @@
+import { Eltro as t, assert} from 'eltro'
+import lowdb from '../core/db.mjs'

test/template.mjs

@@ -0,0 +1 @@
+export default { a: 1 }

test/util.test.mjs

@@ -0,0 +1,40 @@
+import { Eltro as t, assert} from 'eltro'
+import fs from 'fs/promises'
+import Util from '../core/util.mjs'
+
+const isWindows = process.platform === 'win32'
+
+t.describe('#getPathFromRoot()', function() {
+  t.test('should return file relative to root', async function() {
+    var util = new Util(import.meta.url)
+    let path = util.getPathFromRoot('')
+    if (isWindows) {
+      assert.ok(path.endsWith('\\test\\'))
+    } else {
+      assert.ok(path.endsWith('/test/'))
+    }
+
+    path = util.getPathFromRoot('../core/http.mjs')
+    if (isWindows) {
+      assert.ok(path.endsWith('\\core\\http.mjs'))
+    } else {
+      assert.ok(path.endsWith('/core/http.mjs'))
+    }
+
+    let stat = await fs.stat(util.getPathFromRoot('../manage/.gitkeep'))
+    assert.strictEqual(stat.size, 0)
+  })
+})
+
+t.describe('#getUrlFromRoot()', function() {
+  t.test('should return an import compatible path', async function() {
+    var util = new Util(import.meta.url)
+    let err = await assert.isRejected(import(util.getPathFromRoot('template.mjs')))
+    assert.match(err.message, /ESM/i)
+    assert.match(err.message, /file/i)
+    assert.match(err.message, /data/i)
+
+    let data = await import(util.getUrlFromRoot('template.mjs'))
+    assert.deepEqual(data.default, { a: 1 })
+  })
+})