Clean up lint warnings (#547)

* Simplify fauna local argv validation
* Simplify request handling in OAuthClient
* Lower complexity of query info formatting
* Move all TODOs to issues we can track separately

src/cli.mjs

@@ -141,7 +141,7 @@ function buildYargs(argvInput) {
           .filter((key) => previousWord === key)
           .pop();
 
-        // TODO: this doesn't handle aliasing, and it needs to
+        // This doesn't handle aliasing, and it needs to
         if (
           currentWord === "--profile" ||
           currentWordFlag === "profile" ||

src/commands/local.mjs

@@ -124,6 +124,32 @@ ${chalk.red("Please use choose a different name using --name or align the --type
   }
 }
 
+function validateContainerArgv(argv) {
+  if (argv.maxAttempts < 1) {
+    throw new ValidationError("--max-attempts must be greater than 0.");
+  }
+  if (argv.interval < 0) {
+    throw new ValidationError("--interval must be greater than or equal to 0.");
+  }
+}
+
+function validateDatabaseArgv(argv) {
+  const dbOnlyArgs = {
+    typechecked: "--typechecked",
+    protected: "--protected",
+    priority: "--priority",
+    directory: "--fsl-directory",
+  };
+
+  for (const [arg, name] of Object.entries(dbOnlyArgs)) {
+    if (argv[arg] !== undefined && !argv.database) {
+      throw new ValidationError(
+        `${name} can only be set if --database is set.`,
+      );
+    }
+  }
+}
+
 /**
  * Builds the yargs command for the local command
  * @param {import('yargs').Argv} yargs The yargs instance
@@ -198,34 +224,8 @@ function buildLocalCommand(yargs) {
       },
     })
     .check((argv) => {
-      if (argv.maxAttempts < 1) {
-        throw new ValidationError("--max-attempts must be greater than 0.");
-      }
-      if (argv.interval < 0) {
-        throw new ValidationError(
-          "--interval must be greater than or equal to 0.",
-        );
-      }
-      if (argv.typechecked !== undefined && !argv.database) {
-        throw new ValidationError(
-          "--typechecked can only be set if --database is set.",
-        );
-      }
-      if (argv.protected && !argv.database) {
-        throw new ValidationError(
-          "--protected can only be set if --database is set.",
-        );
-      }
-      if (argv.priority && !argv.database) {
-        throw new ValidationError(
-          "--priority can only be set if --database is set.",
-        );
-      }
-      if (argv.directory && !argv.database) {
-        throw new ValidationError(
-          "--directory,--dir can only be set if --database is set.",
-        );
-      }
+      validateContainerArgv(argv);
+      validateDatabaseArgv(argv);
       return true;
     })
     .example([

src/commands/shell.mjs

@@ -35,7 +35,6 @@ async function shellCommand(argv) {
     prompt: `${argv.database || ""}> `,
     ignoreUndefined: true,
     preview: argv.apiVersion !== "10",
-    // TODO: integrate with fql-analyzer for completions
     completer: argv.apiVersion === "10" ? () => [] : undefined,
     output: container.resolve("stdoutStream"),
     input: container.resolve("stdinStream"),

src/lib/auth/accountKeys.mjs

@@ -81,7 +81,7 @@ export class AccountKeys {
   async getOrRefreshKey() {
     if (this.keySource === "credentials-file") {
       const key = this.keyStore.get();
-      // TODO: track ttl for account and refresh keys
+
       if (!key) {
         this.logger.debug(
           "Found account key, but it is expired. Refreshing...",

src/lib/auth/credentials.mjs

@@ -64,7 +64,6 @@ export class Credentials {
     this.accountKeys.keyStore.save({
       accountKey,
       refreshToken,
-      // TODO: set expiration
     });
     this.accountKeys.key = accountKey;
   }

src/lib/auth/oauth-client.mjs

@@ -1,10 +1,19 @@
 import { createHash, randomBytes } from "crypto";
 import http from "http";
 import url from "url";
+import util from "util";
 
 import { container } from "../../cli.mjs";
 import SuccessPage from "./successPage.mjs";
 
+const ALLOWED_ORIGINS = [
+  "http://localhost:3005",
+  "http://127.0.0.1:3005",
+  "http://dashboard.fauna.com",
+  "http://dashboard.fauna-dev.com",
+  "http://dashboard.fauna-preview.com",
+];
+
 // Default to prod client id and secret
 const CLIENT_ID = process.env.FAUNA_CLIENT_ID ?? "Aq4_G0mOtm_F1fK3PuzE0k-i9F0";
 // Native public clients are not confidential. The client secret is not used beyond
@@ -52,64 +61,73 @@ class OAuthClient {
     return Buffer.from(randomBytes(20)).toString("base64url");
   }
 
+  _handleRedirect({ pathname, res }) {
+    if (pathname === "/success") {
+      res.writeHead(200, { "Content-Type": "text/html" });
+      res.write(SuccessPage);
+      res.end();
+      this.closeServer();
+    } else if (pathname !== "/") {
+      throw new Error("Invalid redirect uri");
+    }
+  }
+
+  _handleCode({ authCode, state, res }) {
+    if (!authCode || typeof authCode !== "string") {
+      throw new Error("Invalid authorization code received");
+    } else {
+      this.authCode = authCode;
+      if (state !== this.state) {
+        throw new Error("Invalid state received");
+      }
+      res.writeHead(302, { Location: "/success" });
+      res.end();
+      this.server.emit("auth_code_received");
+    }
+  }
+
   // req: IncomingMessage, res: ServerResponse
   _handleRequest(req, res) {
     const logger = container.resolve("logger");
-    const allowedOrigins = [
-      "http://localhost:3005",
-      "http://127.0.0.1:3005",
-      "http://dashboard.fauna.com",
-      "http://dashboard.fauna-dev.com",
-      "http://dashboard.fauna-preview.com",
-    ];
     const origin = req.headers.origin || "";
 
-    if (allowedOrigins.includes(origin)) {
+    if (ALLOWED_ORIGINS.includes(origin)) {
       res.setHeader("Access-Control-Allow-Origin", origin);
       res.setHeader("Access-Control-Allow-Methods", "GET");
       res.setHeader("Access-Control-Allow-Headers", "Content-Type");
     }
 
-    let errorMessage = "";
-
-    if (req.method === "GET") {
-      const parsedUrl = url.parse(req.url || "", true);
-      if (parsedUrl.pathname === "/success") {
-        res.writeHead(200, { "Content-Type": "text/html" });
-        res.write(SuccessPage);
-        res.end();
-        this.closeServer();
-      } else if (parsedUrl.pathname !== "/") {
-        errorMessage = "Invalid redirect uri";
-        this.closeServer();
-      }
-      const query = parsedUrl.query;
-      if (query.error) {
-        errorMessage = `${query.error.toString()} - ${query.error_description}`;
-        this.closeServer();
-      }
-      if (query.code) {
-        const authCode = query.code;
-        if (!authCode || typeof authCode !== "string") {
-          errorMessage = "Invalid authorization code received";
-          this.server.close();
-        } else {
-          this.authCode = authCode;
-          if (query.state !== this.state) {
-            errorMessage = "Invalid state received";
-            this.closeServer();
-          }
-          res.writeHead(302, { Location: "/success" });
-          res.end();
-          this.server.emit("auth_code_received");
+    try {
+      // We only expect GET requests
+      if (req.method === "GET") {
+        const { pathname, query } = url.parse(req.url || "", true);
+
+        // If the pathname is not "/", we're handling a redirect
+        if (pathname !== "/") {
+          this._handleRedirect({ pathname, res });
+        }
+
+        // If the query contains an error, we're handling an error
+        if (query.error) {
+          throw new Error(
+            `${query.error.toString()} - ${query.error_description}`,
+          );
         }
+
+        // If the query contains an auth code, we're handling a successful auth
+        if (query.code) {
+          this._handleCode({ authCode: query.code, state: query.state, res });
+        }
+      } else {
+        throw new Error("Invalid request method");
       }
-    } else {
-      errorMessage = "Invalid request method";
+    } catch (e) {
       this.closeServer();
-    }
-    if (errorMessage) {
-      logger.stderr(`Error during authentication: ${errorMessage}`);
+      logger.debug(
+        `Authentication error: ${util.inspect(e, true, 2, false)}`,
+        "creds",
+      );
+      logger.stderr(`Error during authentication: ${e.message}`);
     }
   }
 

src/lib/fauna-account-client.mjs

@@ -131,8 +131,6 @@ export class FaunaAccountClient {
    * @returns {Promise<{accountKey: string, refreshToken: string}>} - The session information.
    * @throws {Error} - Throws an error if there is an issue during session retrieval.
    */
-
-  // TODO: get/set expiration details
   static async getSession(accessToken) {
     const makeAccountRequest = container.resolve("makeAccountRequest");
     try {
@@ -149,7 +147,6 @@ export class FaunaAccountClient {
     }
   }
 
-  // TODO: get/set expiration details
   /**
    * Uses refreshToken to get a new accountKey and refreshToken.
    * @param {*} refreshToken

src/lib/fauna-client.mjs

@@ -187,26 +187,29 @@ export const formatQuerySummary = (summary) => {
   }
 };
 
-/**
- * Selects a subset of query info fields from a v10 query response.
- * @param {import("fauna").QueryInfo} response - The query response
- * @param {string[]} include - The query info fields to include
- * @returns {object} An object with the selected query info fields
- */
-const pickAndCamelCaseQueryInfo = (response, include) => {
-  const queryInfo = {};
-
-  if (include.includes("txnTs") && response.txn_ts)
-    queryInfo.txnTs = response.txn_ts;
-  if (include.includes("schemaVersion") && response.schema_version)
-    queryInfo.schemaVersion = response.schema_version.toString();
-  if (include.includes("summary") && response.summary)
-    queryInfo.summary = response.summary;
-  if (include.includes("queryTags") && response.query_tags)
-    queryInfo.queryTags = response.query_tags;
-  if (include.includes("stats") && response.stats)
-    queryInfo.stats = response.stats;
+const getQueryInfoValue = (response, field) => {
+  switch (field) {
+    case "txnTs":
+      return response.txn_ts;
+    case "schemaVersion":
+      return response.schema_version?.toString();
+    case "summary":
+      return response.summary;
+    case "queryTags":
+      return response.query_tags;
+    case "stats":
+      return response.stats;
+    default:
+      return undefined;
+  }
+};
 
+const getIncludedQueryInfo = (response, include) => {
+  const queryInfo = {};
+  include.forEach((field) => {
+    const value = getQueryInfoValue(response, field);
+    if (value) queryInfo[field] = value;
+  });
   return queryInfo;
 };
 
@@ -230,7 +233,7 @@ export const formatQueryInfo = (response, { apiVersion, color, include }) => {
 
     return `${colorized}\n`;
   } else if (apiVersion === "10") {
-    const queryInfoToDisplay = pickAndCamelCaseQueryInfo(response, include);
+    const queryInfoToDisplay = getIncludedQueryInfo(response, include);
 
     if (Object.keys(queryInfoToDisplay).length === 0) return "";
 

src/lib/middleware.mjs

@@ -43,7 +43,6 @@ export function fixPaths(argv) {
 }
 
 export function checkForUpdates(argv) {
-  // TODO: figure out upgrade path for SEA installations
   if (isSea()) return argv;
 
   const __filename = fileURLToPath(import.meta.url);

test/schema/commit.mjs

@@ -91,7 +91,6 @@ describe("schema commit", function () {
   });
 
   it("errors if the schema is not in a ready state", async function () {
-    // TODO: what are the valid statuses? !none, !ready results in this case
     fetch.onCall(0).resolves(f({ status: "building", diff: diff }));
 
     const [error] = await tryToCatch(() =>