feat: better application logging, including errors with stacktraces (#18

)
liatrio-enterprise · Feb 25, 2022 · 93987b5 · 93987b5
1 parent 7c42657
commit 93987b5
Show file tree

Hide file tree

Showing 5 changed files with 182 additions and 58 deletions.
diff --git a/package.json b/package.json
@@ -4,7 +4,8 @@
   "description": "",
   "main": "src/app.js",
   "scripts": {
-    "lint": "eslint ."
+    "lint": "eslint .",
+    "lintfix": "eslint --fix ."
   },
   "repository": {
     "type": "git",
@@ -26,6 +27,7 @@
   },
   "devDependencies": {
     "@liatrio/eslint-config": "^1.0.1",
-    "eslint": "^8.9.0"
+    "eslint": "^8.9.0",
+    "pino-pretty": "^7.5.1"
   }
 }
diff --git a/src/app.js b/src/app.js
@@ -1,23 +1,62 @@
 const { App, createNodeMiddleware } = require("@octokit/app");
 const express = require("express");
-const logger = require("pino")();
+const fs = require("node:fs");
+
+const pino = require("pino");
 const expressLogger = require("express-pino-logger");
 
 const branchProtection = require("./handlers/branch-protection");
 const repoTeamManager = require("./handlers/repo-team-manager");
 
+const getLogger = () => {
+    const pinoOptions = {
+        serializers: {
+            err: pino.stdSerializers.err,
+            error: pino.stdSerializers.err,
+        },
+    };
+
+    if (process.env.NODE_ENV === "local") {
+        // eslint will get mad about this, because pino-pretty is a devDependency
+        // however, this code should only execute when running locally anyways
+        // eslint-disable-next-line node/no-unpublished-require
+        const pretty = require("pino-pretty");
+
+        return pino(pinoOptions, pretty({
+            colorize: true,
+        }));
+    }
+
+    return pino(pinoOptions);
+};
+
+const logger = getLogger();
+
 const expressApp = express();
 
 expressApp.use(express.json());
 expressApp.use(expressLogger({
-    logger
+    logger,
 }));
 
 require("dotenv").config();
 
+const getAppPrivateKey = () => {
+    if (process.env.GITHUB_APP_PRIVATE_KEY) {
+        return process.env.GITHUB_APP_PRIVATE_KEY;
+    }
+
+    if (process.env.GITHUB_APP_PRIVATE_KEY_PATH) {
+        // eslint-disable-next-line security/detect-non-literal-fs-filename
+        return fs.readFileSync(process.env.GITHUB_APP_PRIVATE_KEY_PATH).toString();
+    }
+
+    return undefined;
+};
+
 const app = new App({
     appId: process.env.GITHUB_APP_ID,
-    privateKey: process.env.GITHUB_APP_PRIVATE_KEY,
+    privateKey: getAppPrivateKey(),
     oauth: {
         clientId: process.env.GITHUB_APP_CLIENT_ID,
         clientSecret: process.env.GITHUB_APP_CLIENT_SECRET,

diff --git a/src/handlers/branch-protection.js b/src/handlers/branch-protection.js
@@ -2,27 +2,33 @@ module.exports = (logger) => async ({ octokit, payload }) => {
     if (payload.sender.type !== "Bot") {
         logger.info("Enabling branch protection");
 
-        await octokit.request(
-            "PUT /repos/{owner}/{repo}/branches/{branch}/protection",
-            {
-                owner: payload.repository.owner.login,
-                repo: payload.repository.name,
-                branch: payload.repository.default_branch,
-                required_status_checks: {
-                    contexts: [],
-                    strict: true,
+        try {
+            await octokit.request(
+                "PUT /repos/{owner}/{repo}/branches/{branch}/protection",
+                {
+                    owner: payload.repository.owner.login,
+                    repo: payload.repository.name,
+                    branch: payload.repository.default_branch,
+                    required_status_checks: {
+                        contexts: [],
+                        strict: true,
+                    },
+                    enforce_admins: true,
+                    required_pull_request_reviews: {
+                        dismiss_stale_reviews: true,
+                        required_approving_review_count: 1,
+                    },
+                    required_linear_history: true,
+                    allow_force_pushes: false,
+                    allow_deletions: false,
+                    required_conversation_resolution: true,
+                    restrictions: null, // eslint-disable-line unicorn/no-null
                 },
-                enforce_admins: true,
-                required_pull_request_reviews: {
-                    dismiss_stale_reviews: true,
-                    required_approving_review_count: 1,
-                },
-                required_linear_history: true,
-                allow_force_pushes: false,
-                allow_deletions: false,
-                required_conversation_resolution: true,
-                restrictions: undefined,
-            },
-        );
+            );
+        } catch (error) {
+            logger.error({ error }, "Error enabling branch protection");
+
+            throw error;
+        }
     }
 };
diff --git a/src/handlers/repo-team-manager.js b/src/handlers/repo-team-manager.js
@@ -6,34 +6,30 @@ module.exports = (logger) => async ({ octokit, payload }) => {
         return;
     }
 
-    const teamSlug = repoName.split(".")[0];
+    const [teamSlug] = repoName.split(".");
 
-    const teamResponse = await octokit.request(
-        "GET /orgs/{org}/teams/{teamSlug}",
-        {
-            org: payload.repository.owner.login,
-            teamSlug,
-        },
-    ).catch((error) => {
-        logger.info(error);
-    });
+    try {
+        const teamResponse = await octokit.request(
+            "GET /orgs/{org}/teams/{teamSlug}",
+            {
+                org: payload.repository.owner.login,
+                teamSlug,
+            },
+        );
 
-    if (teamResponse?.status !== 200) {
-        logger.info(`Team response: ${teamResponse?.status}`);
+        await octokit.request(
+            "PUT /orgs/{org}/teams/{teamSlug}/repos/{owner}/{repo}",
+            {
+                org: payload.repository.owner.login,
+                teamSlug: teamResponse.data.slug,
+                owner: payload.repository.owner.login,
+                permission: "maintain",
+                repo: repoName,
+            },
+        );
+    } catch (error) {
+        logger.error({ error });
 
-        return;
+        throw error;
     }
-
-    await octokit.request(
-        "PUT /orgs/{org}/teams/{teamSlug}/repos/{owner}/{repo}",
-        {
-            org: payload.repository.owner.login,
-            teamSlug: teamResponse.data.slug,
-            owner: payload.repository.owner.login,
-            permission: "maintain",
-            repo: repoName,
-        },
-    ).catch((error) => {
-        logger.error(error);
-    });
 };