Improved backend error handling

4 files changed, 50 insertions, 65 deletions

diff --git a/packages/server/src/api/git.ts b/packages/server/src/api/git.ts
index 8d532b0..cebaddc 100644
--- a/packages/server/src/api/git.ts
+++ b/packages/server/src/api/git.ts
@@ -1,4 +1,4 @@
-import { Commit, ConvenientHunk, ConvenientPatch, Object, Repository, Revwalk, Tag, TreeEntry } from "nodegit";
+import { Commit, ConvenientHunk, ConvenientPatch, Object, Oid, Repository, Revwalk, Tag, TreeEntry } from "nodegit";
 import { FastifyReply, FastifyRequest } from "fastify";
 import { join, parse } from "path";
 import { readFile, readdir } from "fs";
@@ -380,7 +380,7 @@ export class GitAPI {
 		const request_info = getRequestInfo(req);
 
 		const valid_request = verifyGitRequest(request_info);
-		if(valid_request.success === false) {
+		if(valid_request.success === false && valid_request.code) {
 			reply.header("Content-Type", request_info.content_type);
 			reply.code(valid_request.code).send(valid_request.message);
 			return;
@@ -431,13 +431,13 @@ export class GitAPI {
 					return { type: "blob", content: (await path_entry.getBlob()).content().toString() };
 				}
 
-				entries = await (await path_entry.getTree()).entries();
+				entries = (await path_entry.getTree()).entries();
 			}
 			catch(err) {
 				if(err.errno === -3) {
-					return { error: 404 };
+					return null;
 				}
-				return { error: 500 };
+				throw(err);
 			}
 		}
 		else {
@@ -466,17 +466,13 @@ export class GitAPI {
 		};
 	}
 
-	async doesCommitExist(repo_name: string, commit_oid: string) {
+	async doesObjectExist(repo_name: string, id: string) {
 		const full_repo_name = addRepoDirSuffix(repo_name);
 		const repo = await Repository.openBare(`${this.base_dir}/${full_repo_name}`);
 
-		try {
-			await repo.getCommit(commit_oid);
-			return true;
-		}
-		catch {
-			return false;
-		}
+		return Object.lookup(repo, Oid.fromString(id), Object.TYPE.ANY)
+			.then(() => true)
+			.catch(() => false);
 	}
 
 	async doesReadmeExist(repo_name: string) {
diff --git a/packages/server/src/api/util.ts b/packages/server/src/api/util.ts
index 72c8992..49144d4 100644
--- a/packages/server/src/api/util.ts
+++ b/packages/server/src/api/util.ts
@@ -1,28 +1,26 @@
 import { Git, GitAPI } from "./git";
 import { readdir } from "fs";
 
-type VerificationResultErrorType = "REPO_NOT_FOUND" | "REPO_INVALID" | "COMMIT_NOT_FOUND" | "COMMIT_INVALID" | "ACCESS_DENIED";
-
-const verification_error_types = {
-	REPO_NOT_FOUND: { code: 404, message: "Repository not found!" },
-	REPO_INVALID: { code: 403, message: "Invalid repository!" },
-	COMMIT_NOT_FOUND: { code: 404, message: "Commit not found!" },
-	COMMIT_INVALID: { code: 403, message: "Invalid commit!" },
-	ACCESS_DENIED: { code: 403, message: "Access denied!" }
-};
+type VerificationResultType = "SUCCESS" | "NOT_FOUND" | "INVALID" | "ACCESS_DENIED";
 
 export class VerificationResult {
-	constructor(success: boolean, error_type?: VerificationResultErrorType) {
-		this.success = success;
-
-		if(error_type) {
-			this.message = verification_error_types[error_type].message;
-			this.code = verification_error_types[error_type].code;
+	constructor(result: VerificationResultType, subject?: string) {
+		this.success = result === "SUCCESS" ? true : false;
+
+		if(result !== "SUCCESS") {
+			const verification_error_types = {
+				NOT_FOUND: { code: 404, message: `${ String(subject?.substr(0, 1).toUpperCase()) + subject?.substr(1)} not found!` },
+				INVALID: { code: 403, message: `Invalid ${subject}` },
+				ACCESS_DENIED: { code: 403, message: "Access denied!" }
+			};
+
+			this.message = verification_error_types[result].message;
+			this.code = verification_error_types[result].code;
 		}
 	}
 
 	success: boolean;
-	code: number = 0;
+	code: number | null = null;
 	message: string | null = null;
 }
 
@@ -31,49 +29,49 @@ export function verifyRepoName(base_dir: string, repo_name: string) {
 		console.log(repo_name);
 		const is_valid_repo_name = (/^[a-zA-Z0-9.\-_]+$/u).test(repo_name);
 		if(!is_valid_repo_name) {
-			resolve(new VerificationResult(false, "REPO_INVALID"));
+			resolve(new VerificationResult("INVALID", "repository"));
 			return;
 		}
 
 		readdir(base_dir, (err, dir_content) => {
 			if(err) {
-				resolve(new VerificationResult(false, "REPO_NOT_FOUND"));
+				resolve(new VerificationResult("NOT_FOUND", "repository"));
 				return;
 			}
 
 			const dir_content_repos = dir_content.filter(repo => repo.endsWith(".git"));
 			if(!dir_content_repos.includes(repo_name + ".git")) {
-				resolve(new VerificationResult(false, "REPO_NOT_FOUND"));
+				resolve(new VerificationResult("NOT_FOUND", "repository"));
 				return;
 			}
 
-			resolve(new VerificationResult(true));
+			resolve(new VerificationResult("SUCCESS"));
 		});
 	});
 }
 
-export async function verifyCommitID(git: GitAPI, repo: string, commit_id: string) {
-	if(!(/^[a-fA-F0-9]+$/u).test(commit_id)) {
-		return new VerificationResult(false, "COMMIT_INVALID");
+export async function verifySHA(git: GitAPI, repo_name: string, sha: string) {
+	if(!(/^[a-fA-F0-9]+$/u).test(sha)) {
+		return new VerificationResult("INVALID", "sha");
 	}
 
-	const commit_exists = await git.doesCommitExist(repo, commit_id);
+	const object_exists = await git.doesObjectExist(repo_name, sha);
 
-	if(!commit_exists) {
-		return new VerificationResult(false, "COMMIT_NOT_FOUND");
+	if(!object_exists) {
+		return new VerificationResult("NOT_FOUND", "object");
 	}
 
-	return new VerificationResult(true);
+	return new VerificationResult("SUCCESS");
 }
 
-export function verifyGitRequest(request_info: Git.RequestInfo): VerificationResult {
+export function verifyGitRequest(request_info: Git.RequestInfo) {
 	if((/\.\/|\.\./u).test(request_info.parsed_url.pathname)) {
-		return new VerificationResult(false, "REPO_NOT_FOUND");
+		return new VerificationResult("INVALID", "path");
 	}
 
 	if(request_info.service !== "git-upload-pack") {
-		return new VerificationResult(false, "ACCESS_DENIED");
+		return new VerificationResult("ACCESS_DENIED");
 	}
 
-	return new VerificationResult(true);
+	return new VerificationResult("SUCCESS");
 }
\ No newline at end of file
diff --git a/packages/server/src/api/v1.ts b/packages/server/src/api/v1.ts
index b75c473..216d33d 100644
--- a/packages/server/src/api/v1.ts
+++ b/packages/server/src/api/v1.ts
@@ -1,5 +1,5 @@
 import { FastifyInstance, FastifyPluginOptions } from "fastify";
-import { verifyCommitID, verifyRepoName } from "./util";
+import { verifySHA, verifyRepoName } from "./util";
 import { GitAPI } from "./git";
 /* eslint-disable max-lines-per-function */
 
@@ -26,11 +26,6 @@ export default function(fastify: FastifyInstance, opts: FastifyPluginOptions, do
 		handler: async(req, reply) => {
 			const repos = await git.getRepositories();
 
-			if(!repos) {
-				reply.code(500).send({ error: "Internal server error!" });
-				return;
-			}
-
 			reply.send({ data: repos });
 		}
 	});
@@ -41,8 +36,8 @@ export default function(fastify: FastifyInstance, opts: FastifyPluginOptions, do
 		handler: async(req, reply) => {
 			const params: any = req.params;
 			const repo_verification = await verifyRepoName(opts.config.settings.base_dir, params.repo);
-			if(repo_verification.success === false) {
-				reply.code(repo_verification.code).send(repo_verification.message);
+			if(repo_verification.success === false && repo_verification.code) {
+				reply.code(repo_verification.code).send({ error: repo_verification.message });
 			}
 
 			const desc = await git.getRepositoryFile(params.repo, "description");
@@ -55,7 +50,7 @@ export default function(fastify: FastifyInstance, opts: FastifyPluginOptions, do
 		fastify_repo.addHook("onRequest", async(req, reply) => {
 			const params: any = req.params;
 			const repo_verification = await verifyRepoName(opts.config.settings.base_dir, params.repo);
-			if(repo_verification.success === false) {
+			if(repo_verification.success === false && repo_verification.code) {
 				reply.code(repo_verification.code).send({ error: repo_verification.message });
 			}
 		});
@@ -80,9 +75,9 @@ export default function(fastify: FastifyInstance, opts: FastifyPluginOptions, do
 			url: "/log/:commit",
 			handler: async(req, reply) => {
 				const params: any = req.params;
-				const commit_verification = await verifyCommitID(git, params.repo, params.commit);
-				if(commit_verification.success === false) {
-					reply.code(commit_verification.code).send(commit_verification.message);
+				const commit_verification = await verifySHA(git, params.repo, params.commit);
+				if(commit_verification.success === false && commit_verification.code) {
+					reply.code(commit_verification.code).send({ error: commit_verification.message });
 				}
 
 				const commit = await git.getCommit(params.repo, params.commit);
@@ -102,14 +97,10 @@ export default function(fastify: FastifyInstance, opts: FastifyPluginOptions, do
 
 				const tree = await git.getTree(params.repo, tree_path);
 
-				if(tree.error) {
-					if(tree.error === 404) {
-						reply.code(404).send({ error: "Path not found" });
-					}
-					else {
-						reply.code(500).send({ error: "Internal server error" });
-					}
+				if(!tree) {
+					reply.code(404).send({ error: "Path not found" });
 				}
+
 				reply.send({ data: tree });
 			}
 		});
diff --git a/packages/server/src/app.ts b/packages/server/src/app.ts
index 263703b..3cb1c07 100644
--- a/packages/server/src/app.ts
+++ b/packages/server/src/app.ts
@@ -81,7 +81,7 @@ fastify.route({
 		reply.header("Content-Type", "application/x-git-upload-pack-advertisement");
 
 		const repo_verification = await verifyRepoName(settings.base_dir, (<any>req).params.repo);
-		if(repo_verification.success === false) {
+		if(repo_verification.success === false && repo_verification.code) {
 			reply.code(repo_verification.code).send(repo_verification.message);
 		}
 
@@ -108,7 +108,7 @@ fastify.route({
 	url: "/:repo([a-zA-Z0-9\\.\\-_]+)/git-upload-pack",
 	handler: async(req, reply) => {
 		const repo_verification = await verifyRepoName(settings.base_dir, (<any>req).params.repo);
-		if(repo_verification.success === false) {
+		if(repo_verification.success === false && repo_verification.code) {
 			reply.header("Content-Type", "application/x-git-upload-pack-result");
 			reply.code(repo_verification.code).send(repo_verification.message);
 		}