diff options
| author | Kevin J Hoerr <kjhoerr@protonmail.com> | 2019-12-08 14:17:23 -0500 |
|---|---|---|
| committer | Kevin J Hoerr <kjhoerr@protonmail.com> | 2019-12-08 14:17:23 -0500 |
| commit | 3205965afbd9c4fce00dc03cccd6b6223ddb1d15 (patch) | |
| tree | 1c938e39ae5e6ca6d9fca87965b405a30aa8d0f1 | |
| parent | ad627b0c6ba9bc804ed2924c692c8ef7e022fca6 (diff) | |
| download | ao-coverage-3205965afbd9c4fce00dc03cccd6b6223ddb1d15.tar.gz ao-coverage-3205965afbd9c4fce00dc03cccd6b6223ddb1d15.tar.bz2 ao-coverage-3205965afbd9c4fce00dc03cccd6b6223ddb1d15.zip | |
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.
| -rw-r--r-- | CHANGELOG.md | 12 | ||||
| -rw-r--r-- | package.json | 2 | ||||
| -rw-r--r-- | src/errors.ts | 13 | ||||
| -rw-r--r-- | src/formats.ts | 6 | ||||
| -rw-r--r-- | src/index.ts | 1 | ||||
| -rw-r--r-- | src/metadata.ts | 5 | ||||
| -rw-r--r-- | src/routes.ts | 77 |
7 files changed, 88 insertions, 28 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index e69a885..c81d077 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.2.1] + +### Added + +- Business logic errors for explicit dataflow handling + +### Changed + +- Changed formats/metadata to return union types with business logic errors +- Fixed 404 responses for all GET endpoints + ## [0.2.0] ### Added @@ -31,5 +42,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Code formatting using Prettier [unreleased]: https://git.submelon.dev/kjhoerr/ao-coverage/src/branch/trunk +[0.2.1]: https://git.submelon.dev/kjhoerr/ao-coverage/src/tag/v0.2.1 [0.2.0]: https://git.submelon.dev/kjhoerr/ao-coverage/src/tag/v0.2.0 [0.1.0]: https://git.submelon.dev/kjhoerr/ao-coverage/src/tag/v0.1.0 diff --git a/package.json b/package.json index 426909d..7b468f7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "ao-coverage", - "version": "0.2.0", + "version": "0.2.1", "description": "Simple code coverage storage and server", "repository": { "type": "git", 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; |
