Overhaul error handling for controllers

This change fixes various issues with the GET endpoints to return more
accurate error messages. It adds business logic error objects that are
returned in a union type in the controllers. This change should help
separate the business logic from the actual errors from upstream
services.

There may be required changes still with handling those upstream errors
via Promises. Integration tests should be added to verify issues.

5 files changed, 75 insertions, 27 deletions

diff --git a/src/errors.ts b/src/errors.ts
new file mode 100644
index 0000000..4026164
--- /dev/null
+++ b/src/errors.ts
@@ -0,0 +1,13 @@
+export class BranchNotFoundError extends Error {
+  constructor() {
+    super();
+    this.message = "Branch not found";
+  }
+}
+
+export class InvalidReportDocumentError extends Error {
+  constructor() {
+    super();
+    this.message = "Invalid report document";
+  }
+}
diff --git a/src/formats.ts b/src/formats.ts
index 59a966e..1b5a572 100644
--- a/src/formats.ts
+++ b/src/formats.ts
@@ -1,6 +1,8 @@
+import { InvalidReportDocumentError } from "./errors";
+
 export interface Format {
   // returns the coverage value as %: Number(90.0), Number(100.0), Number(89.5)
-  parse_coverage: (file: Document) => number;
+  parse_coverage: (file: Document) => number | InvalidReportDocumentError;
   match_color: (coverage: number, stage_1: number, stage_2: number) => string;
 }
 
@@ -35,7 +37,7 @@ const FormatsObj: FormatObj = {
       parse_coverage: (file: Document) => {
         const scripts = file.getElementsByTagName("script");
         if (scripts.length == 0) {
-          throw new Error("Invalid report document");
+          return new InvalidReportDocumentError();
         }
         const data = scripts[0].text;
         const accumFunc = (regex: RegExp) => {
diff --git a/src/index.ts b/src/index.ts
index 7bf2354..fdcdcaf 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -43,6 +43,7 @@ new MongoClient(MONGO_URI, { useUnifiedTopology: true }).connect(
     app.use(
       expressWinston.logger({
         ...logger_config("HTTP"),
+        colorize: true,
         // filter out token query param from URL
         msg:
           '{{req.method}} {{req.url.replace(/token=[-\\w.~]*(&*)/, "token=$1")}} - {{res.statusCode}} {{res.responseTime}}ms'
diff --git a/src/metadata.ts b/src/metadata.ts
index c297b38..07e420d 100644
--- a/src/metadata.ts
+++ b/src/metadata.ts
@@ -2,6 +2,7 @@ import { Db } from "mongodb";
 import winston from "winston";
 
 import logger_config from "./util/logger";
+import { BranchNotFoundError } from "./errors";
 
 interface Branch {
   head: string;
@@ -37,7 +38,7 @@ class Metadata {
     organization: string,
     repository: string,
     branch: string
-  ): Promise<string> {
+  ): Promise<string | BranchNotFoundError> {
     const result = await this.database
       .collection<Repository>("repository")
       .findOne({
@@ -57,7 +58,7 @@ class Metadata {
       );
       return limb.head;
     } else {
-      throw Error("Branch not found");
+      return new BranchNotFoundError();
     }
   }
 
diff --git a/src/routes.ts b/src/routes.ts
index 2c9cf45..4f622a2 100644
--- a/src/routes.ts
+++ b/src/routes.ts
@@ -7,11 +7,15 @@ import fs from "fs";
 import formats, { Format } from "./formats";
 import Metadata from "./metadata";
 import { config_or_error } from "./util/config";
+import logger_config from "./util/logger";
+import winston from "winston";
 
 const TOKEN = process.env.TOKEN || "";
 const UPLOAD_LIMIT = Number(process.env.UPLOAD_LIMIT || 4194304);
 const HOST_DIR = config_or_error("HOST_DIR");
 
+const logger = winston.createLogger(logger_config("HTTP"));
+
 export default (metadata: Metadata) => {
   const router = express.Router();
 
@@ -38,13 +42,14 @@ export default (metadata: Metadata) => {
       }
     });
     req.on("end", () => {
-      let formatter: Format, coverage: number;
-      try {
-        const doc = new JSDOM(contents).window.document;
-        formatter = formats.get_format(format);
-        coverage = formatter.parse_coverage(doc);
-      } catch {
-        return res.status(400).send("Invalid report document");
+      let coverage: number;
+      const doc = new JSDOM(contents).window.document;
+      const formatter = formats.get_format(format);
+      const result = formatter.parse_coverage(doc);
+      if (typeof result === "number") {
+        coverage = result;
+      } else {
+        return res.status(400).send(result.message);
       }
 
       const badge = badgen({
@@ -84,13 +89,24 @@ export default (metadata: Metadata) => {
     const { org, repo, branch } = req.params;
 
     metadata.getHeadCommit(org, repo, branch).then(
-      commit => {
-        res.sendFile(
-          path.join(HOST_DIR, org, repo, branch, commit, "badge.svg")
-        );
+      result => {
+        if (typeof result === "string") {
+          res.sendFile(
+            path.join(
+              HOST_DIR,
+              org,
+              repo,
+              branch,
+              result.toString(),
+              "badge.svg"
+            )
+          );
+        } else {
+          res.status(404).send(result.message);
+        }
       },
-      () => {
-        //FIXME if ORB DNE, should be 404
+      err => {
+        logger.error(err);
         res.status(500).send("Unknown error occurred");
       }
     );
@@ -100,13 +116,24 @@ export default (metadata: Metadata) => {
     const { org, repo, branch } = req.params;
 
     metadata.getHeadCommit(org, repo, branch).then(
-      commit => {
-        res.sendFile(
-          path.join(HOST_DIR, org, repo, branch, commit, "index.html")
-        );
+      result => {
+        if (typeof result === "string") {
+          res.sendFile(
+            path.join(
+              HOST_DIR,
+              org,
+              repo,
+              branch,
+              result.toString(),
+              "index.html"
+            )
+          );
+        } else {
+          res.status(404).send(result.message);
+        }
       },
-      () => {
-        //FIXME if ORB DNE, should be 404
+      err => {
+        logger.error(err);
         res.status(500).send("Unknown error occurred");
       }
     );
@@ -116,16 +143,20 @@ export default (metadata: Metadata) => {
   router.get("/v1/:org/:repo/:branch/:commit.svg", (req, res) => {
     const { org, repo, branch, commit } = req.params;
 
-    //FIXME prettify error message?
-    res.sendFile(path.join(HOST_DIR, org, repo, branch, commit, "badge.svg"));
+    const file = path.join(HOST_DIR, org, repo, branch, commit, "badge.svg");
+    fs.access(file, fs.constants.R_OK, err =>
+      err === null ? res.sendFile(file) : res.status(404).send("File not found")
+    );
   });
 
   // provide hard link for commit
   router.get("/v1/:org/:repo/:branch/:commit.html", (req, res) => {
     const { org, repo, branch, commit } = req.params;
 
-    //FIXME prettify error message?
-    res.sendFile(path.join(HOST_DIR, org, repo, branch, commit, "index.html"));
+    const file = path.join(HOST_DIR, org, repo, branch, commit, "index.html");
+    fs.access(file, fs.constants.R_OK, err =>
+      err === null ? res.sendFile(file) : res.status(404).send("File not found")
+    );
   });
 
   return router;