Merge pull request #17 from forensic-architecture/fix/error-handling

Fix/error handling closes #2

src/api/index.js

@@ -1,5 +1,6 @@
 import { version } from '../../package.json'
 import { Router } from 'express'
+import copy from '../copy/en'
 
 export default ({ config, controller }) => {
   let api = Router()
@@ -20,8 +21,8 @@ export default ({ config, controller }) => {
       .retrieveFrag(source, tab, resource, frag)
       .then(data => res.json(data))
       .catch(err =>
-        res.status(err.status || 501)
+        res.status(err.status || 404)
-          .send()
+          .send({ error: err.message })
       )
   })
 
@@ -30,8 +31,8 @@ export default ({ config, controller }) => {
       .retrieve(req.params.source, req.params.tab, req.params.resource)
       .then(data => res.json(data))
       .catch(err =>
-        res.status(err.status || 501)
+        res.status(err.status || 404)
-          .send()
+          .send({ error: err.message })
       )
   })
 
@@ -44,11 +45,23 @@ export default ({ config, controller }) => {
         })
       )
       .catch(err =>
-        res.json({
+        res.status(404)
-          error: err.message
+          .send({ error: err.message })
-        })
       )
   })
 
+  // ERROR routes. Note that it is important that these come AFTER routes
+  // like /update, so that the regex does not greedily match these routes.
+
+  api.get('/:source', (req, res) => {
+    res.status(404)
+      .send({ error: copy.errors.onlySource })
+  })
+
+  api.get('/:source/:tab', (req, res) => {
+    res.status(404)
+      .send({ error: copy.errors.onlyTab })
+  })
+
   return api
 }

src/copy/en.js

@@ -0,0 +1,13 @@
+export default {
+  errors: {
+    update: 'The server could not update. Check your API credentials and internet connection and try again.',
+    onlySource: 'You cannot query a source directly. The URL needs to be in the format /:source/:tab/:resource.',
+    onlyTab: 'You cannot query a tab directly. The URL needs to be in the format /:source/:tab/:resource.',
+    noSource: source => `The source ${source} is not available in this server.`,
+    noResource: prts => `The resource '${prts[2]}' does not exists in the tab '${prts[1]}' of the source '${prts[0]}'.`,
+    noFragment: prts => `Fragment index does not exist`
+  },
+  success: {
+    update: 'All sources updated'
+  }
+}

src/lib/Controller.js

@@ -1,3 +1,5 @@
+import copy from '../copy/en'
+
 /**
  * Controller
  *
@@ -23,7 +25,11 @@ class Controller {
         return this.fetchers[source].update()
       })
     ).then(results => {
-      return 'All sources updated'
+      if (results.every(r => r)) {
+        return copy.success.update
+      } else {
+        throw new Error(copy.errors.update)
+      }
     })
   }
 
@@ -32,9 +38,7 @@ class Controller {
       const fetcher = this.fetchers[source]
       return fetcher.retrieve(tab, resource)
     } else {
-      return Promise.resolve().then(() => {
+      return Promise.reject(new Error(copy.errors.noResource(source)))
-        throw new Error(`Source ${source} not available.`)
-      })
     }
   }
 
@@ -43,9 +47,7 @@ class Controller {
       const fetcher = this.fetchers[source]
       return fetcher.retrieveFrag(tab, resource, frag)
     } else {
-      return Promise.resolve().then(() => {
+      return Promise.reject(new Error(copy.errors.noResource(source)))
-        throw new Error(`Source ${source} not available.`)
-      })
     }
   }
 }

src/lib/Fetcher.js

@@ -111,7 +111,8 @@ class Fetcher {
           })
         )
       })
-      .then(() => 'All tabs updated')
+      .then(() => true)
+      .catch(() => false)
   }
 
   save (tab, data) {

src/models/Interface.js

@@ -2,6 +2,12 @@
 /**
  * Model is a class whose sole responsibility is to save and load blueprints.
  * It allows for different storage mechanisms for different kinds of blueprints.
+ * 
+ * ERRORS:
+ * When a load function fails, it should throw either:
+ * 1. A __ error if the resource doesn't exist on that sheet/tab.
+ * 2. A __ error if a fragment lookup fails because it doesn't exist.
+ * 3. A __ error if something else goes wrong.
  */
 class Model {
   /**

src/models/StoreJson.js

@@ -1,5 +1,6 @@
 import fs from 'mz/fs'
 import { fmtSourceTitle } from '../lib/util'
+import copy from '../copy/en'
 
 const STORAGE_DIRNAME = 'temp'
 
@@ -22,14 +23,8 @@ class StoreJson {
     const fname = `${STORAGE_DIRNAME}/${parts[0]}__${parts[1]}__${
       parts[2]
     }.json`
-    return fs
+    if (fs.existsSync(fname)) {
-      .exists(fname)
+      return fs.readFile(fname, 'utf8')
-      .then(isAvailable => {
-        if (isAvailable) return fs.readFile(fname, 'utf8')
-        else {
-          throw new Error('No resource exists')
-        }
-      })
         .then(data => JSON.parse(data))
         .then(data => {
           if (parts.length === 3) {
@@ -42,7 +37,7 @@ class StoreJson {
             if (!isNaN(id) && id >= 0 && id < data.length) {
               return data[id]
             } else {
-            throw new Error(`Fragment index does not exist`)
+              throw new Error(copy.errors.noFragment(parts))
             }
           } else {
             // Do a lookup if fragment is included to filter a relevant item
@@ -51,10 +46,13 @@ class StoreJson {
               console.log(data, index)
               return data.filter((vl, idx) => idx === index)[0]
             } else {
-            throw new Error(`Fragment index does not exist`)
+              throw new Error(copy.errors.noFragment(parts))
             }
           }
         })
+    } else {
+      return Promise.reject(new Error(copy.errors.noResource(parts)))
+    }
   }
 
   // TODO: add method to build blueprint from data source