From 507a72b772ac468674708ce321f9835ef82ec3f9 Mon Sep 17 00:00:00 2001 From: echo-bravo-yahoo Date: Fri, 13 Dec 2024 16:13:52 -0800 Subject: [PATCH 1/3] add bash/zsh completions (#438) this change adds: - the default yargs completions for commands, options, etc. - custom completions for profiles (given that you have a config specified) - custom completions for database path (given that you have specified enough information to make a request to fauna) --- src/cli.mjs | 53 +++++++- src/commands/database/list.mjs | 42 ++++--- src/commands/query.mjs | 3 + src/lib/command-helpers.mjs | 4 + src/lib/completions.mjs | 45 +++++++ src/lib/config/config.mjs | 6 +- src/lib/middleware.mjs | 4 +- test/completions.mjs | 220 +++++++++++++++++++++++++++++++++ test/helpers.mjs | 15 +++ 9 files changed, 363 insertions(+), 29 deletions(-) create mode 100644 src/lib/completions.mjs create mode 100644 test/completions.mjs diff --git a/src/cli.mjs b/src/cli.mjs index a4d70b4a..923744db 100644 --- a/src/cli.mjs +++ b/src/cli.mjs @@ -10,6 +10,7 @@ import queryCommand from "./commands/query.mjs"; import schemaCommand from "./commands/schema/schema.mjs"; import shellCommand from "./commands/shell.mjs"; import { buildCredentials } from "./lib/auth/credentials.mjs"; +import { getDbCompletions, getProfileCompletions } from "./lib/completions.mjs"; import { configParser } from "./lib/config/config.mjs"; import { handleParseYargsError } from "./lib/errors.mjs"; import { @@ -112,6 +113,50 @@ function buildYargs(argvInput) { .command(localCommand) .demandCommand() .strictCommands(true) + .completion( + "completion", + "Output bash/zsh script to enable shell completions. See command output for installation instructions.", + ) + .completion( + "completion", + async function (currentWord, argv, defaultCompletions, done) { + // this is pretty hard to debug - if you need to, run + // `fauna --get-yargs-completions ` + // for example: `fauna --get-yargs-completions --profile he` + // note that you need to have empty quotes to get all matches: + // `fauna --get-yargs-completions --profile ""` + + // then, call the done callback with an array of strings for debugging, like: + // done( + // [ + // `currentWord: ${currentWord}, currentWordFlag: ${currentWordFlag}, argv: ${JSON.stringify(argv)}`, + // ], + // ); + const previousWord = process.argv.slice(-2, -1)[0].replace(/-/g, ""); + const currentWordFlag = Object.keys(argv) + .filter((key) => previousWord === key) + .pop(); + + // TODO: this doesn't handle aliasing, and it needs to + if ( + currentWord === "--profile" || + currentWordFlag === "profile" || + currentWord === "-p" || + currentWordFlag === "p" + ) { + done(getProfileCompletions(currentWord, argv)); + } else if ( + currentWord === "--database" || + currentWordFlag === "database" || + currentWord === "-d" || + currentWordFlag === "d" + ) { + done(await getDbCompletions(currentWord, argv)); + } else { + defaultCompletions(); + } + }, + ) .options({ color: { description: @@ -153,7 +198,7 @@ function buildYargs(argvInput) { "Components to emit diagnostic logs for. Takes precedence over the `--verbosity` flag. Pass components as a space-separated list, such as `--verboseComponent fetch error`, or as separate flags, such as `--verboseComponent fetch --verboseComponent error`.", type: "array", default: [], - choices: ["fetch", "error", "config", "argv", "creds"], + choices: ["fetch", "error", "config", "argv", "creds", "completion"], group: "Debug:", }, verbosity: { @@ -169,9 +214,5 @@ function buildYargs(argvInput) { .alias("help", "h") .fail(false) .exitProcess(false) - .version() - .completion( - "completion", - "Output bash/zsh script to enable shell completions. See command output for installation instructions.", - ); + .version(); } diff --git a/src/commands/database/list.mjs b/src/commands/database/list.mjs index f8845de1..e5c78ac5 100644 --- a/src/commands/database/list.mjs +++ b/src/commands/database/list.mjs @@ -28,38 +28,27 @@ function pickOutputFields(databases, argv) { } async function listDatabasesWithAccountAPI(argv) { - const { pageSize, database, color } = argv; + const { pageSize, database } = argv; const accountClient = new FaunaAccountClient(); const response = await accountClient.listDatabases({ pageSize, path: database, }); - const output = pickOutputFields(response.results, argv); - - container.resolve("logger").stdout( - colorize(output, { - format: Format.JSON, - color: color, - }), - ); + return pickOutputFields(response.results, argv); } async function listDatabasesWithSecret(argv) { - const { url, secret, pageSize, color } = argv; - const { runQueryFromString, formatQueryResponse } = - container.resolve("faunaClientV10"); + const { url, secret, pageSize } = argv; + const { runQueryFromString } = container.resolve("faunaClientV10"); try { - const result = await runQueryFromString({ + return await runQueryFromString({ url, secret, // This gives us back an array of database names. If we want to // provide the after token at some point this query will need to be updated. expression: `Database.all().paginate(${pageSize}).data { ${getOutputFields(argv)} }`, }); - container - .resolve("logger") - .stdout(formatQueryResponse(result, { format: Format.JSON, color })); } catch (e) { if (e instanceof FaunaError) { throwForError(e); @@ -68,11 +57,24 @@ async function listDatabasesWithSecret(argv) { } } -async function listDatabases(argv) { +export async function listDatabases(argv) { + let databases; + if (argv.secret) { + databases = await listDatabasesWithSecret(argv); + } else { + databases = await listDatabasesWithAccountAPI(argv); + } + return databases; +} + +async function doListDatabases(argv) { + const logger = container.resolve("logger"); + const { formatQueryResponse } = container.resolve("faunaClientV10"); + const res = await listDatabases(argv); if (argv.secret) { - return listDatabasesWithSecret(argv); + logger.stdout(formatQueryResponse(res, argv)); } else { - return listDatabasesWithAccountAPI(argv); + logger.stdout(colorize(res, { format: Format.JSON, color: argv.color })); } } @@ -110,5 +112,5 @@ export default { command: "list", description: "List databases.", builder: buildListCommand, - handler: listDatabases, + handler: doListDatabases, }; diff --git a/src/commands/query.mjs b/src/commands/query.mjs index 0f8c3617..f0446210 100644 --- a/src/commands/query.mjs +++ b/src/commands/query.mjs @@ -23,6 +23,9 @@ function validate(argv) { const { existsSync, accessSync, constants } = container.resolve("fs"); const dirname = container.resolve("dirname"); + // don't validate completion invocations + if (argv.getYargsCompletions) return; + if (argv.input && argv.fql) { throw new ValidationError("Cannot specify both --input and [fql]"); } diff --git a/src/lib/command-helpers.mjs b/src/lib/command-helpers.mjs index 28223bc3..9f61b5a4 100644 --- a/src/lib/command-helpers.mjs +++ b/src/lib/command-helpers.mjs @@ -165,8 +165,12 @@ export const resolveFormat = (argv) => { * @param {string} argv.database - The database to use * @param {string} argv.secret - The secret to use * @param {boolean} argv.local - Whether to use a local Fauna container + * @param {boolean|undefined} argv.getYargsCompletions - Whether this CLI run is to generate completions */ export const validateDatabaseOrSecret = (argv) => { + // don't validate completion invocations + if (argv.getYargsCompletions) return true; + if (!argv.database && !argv.secret && !argv.local) { throw new ValidationError( "No database or secret specified. Please use either --database, --secret, or --local to connect to your desired Fauna database.", diff --git a/src/lib/completions.mjs b/src/lib/completions.mjs new file mode 100644 index 00000000..ea1d520e --- /dev/null +++ b/src/lib/completions.mjs @@ -0,0 +1,45 @@ +// @ts-check + +import * as path from "node:path"; + +import { FaunaAccountClient } from "../lib/fauna-account-client.mjs"; +import { buildCredentials } from "./auth/credentials.mjs"; +import { getConfig, locateConfig } from "./config/config.mjs"; + +export function getProfileCompletions(currentWord, argv) { + const configPath = locateConfig(path.resolve(argv.config)); + if (!configPath) return undefined; + return Object.keys(getConfig(configPath).toJSON()); +} + +export async function getDbCompletions(currentWord, argv) { + const regionGroups = ["us-std", "eu-std", "global"]; + + function getRegionGroup(currentWord) { + const rg = regionGroups.filter((rg) => currentWord.startsWith(rg)); + return rg.length ? rg[0] : undefined; + } + + if (!getRegionGroup(currentWord)) { + return regionGroups; + } else { + const { pageSize } = argv; + buildCredentials({ ...argv, user: "default" }); + const accountClient = new FaunaAccountClient(); + try { + const response = await accountClient.listDatabases({ + pageSize, + path: currentWord, + }); + return response.results.map(({ name }) => path.join(currentWord, name)); + } catch (e) { + const response = await accountClient.listDatabases({ + pageSize, + path: path.dirname(currentWord), + }); + return response.results.map(({ name }) => + path.join(path.dirname(currentWord), name), + ); + } + } +} diff --git a/src/lib/config/config.mjs b/src/lib/config/config.mjs index 79324ce6..9f192237 100644 --- a/src/lib/config/config.mjs +++ b/src/lib/config/config.mjs @@ -35,6 +35,10 @@ export function getConfig(path) { return yaml.parseDocument(fileBody); } +export function locateConfig(path) { + return path === process.cwd() ? checkForDefaultConfig(process.cwd()) : path; +} + /** * Checks the specified directory for default configuration files. * @@ -96,7 +100,7 @@ function validateConfig(profileName, profileBody, configPath) { * * @param {string|string[]} argvInput - The raw command line arguments. * @param {string} path - * @returns {object} - The yargs parser + * @returns {object} - The parsed argv */ export function configParser(argvInput, path) { const userProvidedConfigPath = diff --git a/src/lib/middleware.mjs b/src/lib/middleware.mjs index 631ce256..d23d441d 100644 --- a/src/lib/middleware.mjs +++ b/src/lib/middleware.mjs @@ -72,11 +72,11 @@ export function checkForUpdates(argv) { * If --local is provided and --secret is not, argv.secret is * set to 'secret'. * @param {import('yargs').Arguments} argv - * @returns {import('yargs').Arguments} + * @returns {void} */ export function applyLocalArg(argv) { applyLocalToUrl(argv); - return applyLocalToSecret(argv); + applyLocalToSecret(argv); } /** diff --git a/test/completions.mjs b/test/completions.mjs new file mode 100644 index 00000000..9076a83a --- /dev/null +++ b/test/completions.mjs @@ -0,0 +1,220 @@ +//@ts-check + +import { expect } from "chai"; +import { match, stub } from "sinon"; + +import { run } from "../src/cli.mjs"; +import { setupTestContainer as setupContainer } from "../src/config/setup-test-container.mjs"; +import { validDefaultConfigNames } from "../src/lib/config/config.mjs"; +import { eventually, mockAccessKeysFile } from "./helpers.mjs"; + +const realLogger = console.log; // eslint-disable-line no-console +/** + * @param {object} args + * @prop {string} [configPath] + * @prop {string} [matchFlag = ""] - the option/argument/flag before the substring to generate completions for. in `fauna query --profile hello`, the matchFlag is `profile`; do not include the leading `--`. + * @prop {string} [matchSubstring = ""] - the substring generate completions for. in `fauna query --profile hello`, the matchSubstring is `hello`. + * @prop {string} [command] + * @prop {Record} [env] + * @prop {any} container + */ +async function complete({ + configPath, + matchFlag = "", + matchSubstring = "", + container, + command, + env, +}) { + // to test these manually in zsh/bash, do: + // `fauna --get-yargs-completions fauna query --database "us-std/stringToComplete"` + let commandString = `fauna --get-yargs-completions fauna`; + if (command) commandString += ` ${command}`; + if (configPath) commandString += ` --config ${configPath}`; + commandString += ` --${matchFlag} ${matchSubstring}`; + process.argv = commandString.split(" "); + + if (env) { + for (const [key, value] of Object.entries(env)) { + process.env[key] = value; + } + } + + await run(commandString.split(" "), container); +} + +const basicConfig = { + default: {}, + dev: {}, + prod: {}, +}; + +const advancedConfig = { + development: {}, + production: {}, +}; + +const defaultNameConfig = { + plain: {}, + boring: {}, +}; + +describe("shell completion", () => { + let container, fs, fakeLogger; + + beforeEach(() => { + // reset the container before each test + container = setupContainer(); + + fakeLogger = stub(); + console.log = fakeLogger; // eslint-disable-line no-console + fs = container.resolve("fs"); + }); + + after(() => { + console.log = realLogger; // eslint-disable-line no-console + }); + + describe("for profiles", () => { + beforeEach(() => { + fs.readdirSync.withArgs(match.any).returns([ + { + isFile: () => true, + name: validDefaultConfigNames[0], + path: process.cwd(), + parentParth: process.cwd(), + }, + ]); + fs.readFileSync + .withArgs("/config/basic.yaml") + .returns(JSON.stringify(basicConfig)); + fs.readFileSync + .withArgs("/config/advanced.yaml") + .returns(JSON.stringify(advancedConfig)); + fs.readFileSync + .withArgs(validDefaultConfigNames[0]) + .returns(JSON.stringify(defaultNameConfig)); + }); + + it("works with config files in the same directory with default names", async () => { + await complete({ + container, + matchFlag: "profile", + }); + expect(fakeLogger).to.have.been.calledWith("plain"); + expect(fakeLogger).to.have.been.calledWith("boring"); + }); + + it("works with config files chosen by flag or env var", async () => { + await complete({ + container, + matchFlag: "profile", + env: { + FAUNA_CONFIG: "/config/basic.yaml", + }, + }); + expect(fakeLogger).to.have.been.calledWith("default"); + expect(fakeLogger).to.have.been.calledWith("dev"); + expect(fakeLogger).to.have.been.calledWith("prod"); + }); + + it("prioritizes config file paths provided by flag over env vars", async () => { + await complete({ + container, + matchFlag: "profile", + env: { + FAUNA_CONFIG: "/config/basic.yaml", + }, + configPath: "/config/advanced.yaml", + }); + expect(fakeLogger).to.have.been.calledWith("development"); + expect(fakeLogger).to.have.been.calledWith("production"); + }); + + it.skip("is resilient against wrapping quotes", async () => {}); + }); + + describe("for databases", () => { + beforeEach(() => { + // reset the container before each test + container = setupContainer(); + fs = container.resolve("fs"); + fs.readdirSync.withArgs(match.any).returns([ + { + isFile: () => true, + name: validDefaultConfigNames[0], + path: process.cwd(), + parentParth: process.cwd(), + }, + ]); + fs.readFileSync + .withArgs("/config/basic.yaml") + .returns(JSON.stringify(basicConfig)); + mockAccessKeysFile({ fs }); + + let makeAccountRequest = container.resolve("makeAccountRequest"); + const stubbedResponse = { results: [{ name: "americacentric" }] }; + makeAccountRequest + .withArgs(match({ path: "/databases", params: { path: "us-std" } })) + .resolves(stubbedResponse); + }); + + it("suggests a region group if the current word doesn't start with a region group", async () => { + await complete({ + container, + matchFlag: "database", + command: "query", + }); + expect(fakeLogger).to.have.been.calledWith("eu-std"); + expect(fakeLogger).to.have.been.calledWith("us-std"); + expect(fakeLogger).to.have.been.calledWith("global"); + }); + + it("suggests a top level database in the selected region group", async () => { + let makeAccountRequest = container.resolve("makeAccountRequest"); + const stubbedResponse = { results: [{ name: "eurocentric" }] }; + makeAccountRequest + .withArgs(match({ path: "/databases", params: { path: "eu-std" } })) + .resolves(stubbedResponse); + await complete({ + container, + matchFlag: "database", + matchSubstring: "eu-std/", + command: "query", + }); + + await eventually(() => { + expect(fakeLogger).to.have.been.calledWith("eu-std/eurocentric"); + }); + }); + + it("suggests a nested level database in the selected region group", async () => { + let makeAccountRequest = container.resolve("makeAccountRequest"); + const stubbedResponse = { + results: [{ name: "1" }, { name: "2" }, { name: "3" }, { name: "4" }], + }; + makeAccountRequest + .withArgs( + match({ path: "/databases", params: { path: "eu-std/a/b/c/d" } }), + ) + .resolves(stubbedResponse); + + await complete({ + container, + matchFlag: "database", + matchSubstring: "eu-std/a/b/c/d", + command: "query", + }); + await eventually(() => { + expect(fakeLogger).to.have.been.calledWith("eu-std/a/b/c/d/1"); + expect(fakeLogger).to.have.been.calledWith("eu-std/a/b/c/d/2"); + expect(fakeLogger).to.have.been.calledWith("eu-std/a/b/c/d/3"); + expect(fakeLogger).to.have.been.calledWith("eu-std/a/b/c/d/4"); + }); + }); + + it.skip("is resilient against trailing slashes", async () => {}); + + it.skip("is resilient against wrapping quotes", async () => {}); + }); +}); diff --git a/test/helpers.mjs b/test/helpers.mjs index 35c98681..2a6b5453 100644 --- a/test/helpers.mjs +++ b/test/helpers.mjs @@ -208,3 +208,18 @@ export const mockSecretKeysFile = ({ `{${accountKey}: { "${path}:${role}": {"secret": "${secret}", "expiresAt": ${expiresAt}}}}`, ); }; + +/** + * retry an assertion repeatedly until it succeeds + * @param {function} evaluator - any function that throws if a condition isn't met. + * @param {number} [ms=50] - the number of milliseconds to wait for the condition. set it lower than mocha's timeout to re-throw the underlying error and have usable test failure logs. + */ +export async function eventually(evaluator, ms = 50) { + try { + return evaluator(); + } catch (e) { + if (ms <= 0) throw e; + await new Promise((resolve) => setTimeout(resolve, 1)); // eslint-disable-line no-promise-executor-return + return eventually(evaluator, ms - 1); + } +} From c97f35f34c98acc2088a44ed8d4dcadc6445c982 Mon Sep 17 00:00:00 2001 From: Cleve Stuart <90649124+cleve-fauna@users.noreply.github.com> Date: Fri, 13 Dec 2024 18:08:12 -0800 Subject: [PATCH 2/3] Optionally, create a db when starting a local container (#525) * Optionally, create a db when starting a local container * Don't edit create db * Happy path tests * Smaller test * Argument tests * Failure mode tests * Failure mode tests * One more test * use ValidationError --- src/commands/local.mjs | 111 ++++++++++++++++++++-- src/lib/docker-containers.mjs | 2 - test/local.mjs | 172 ++++++++++++++++++---------------- 3 files changed, 193 insertions(+), 92 deletions(-) diff --git a/src/commands/local.mjs b/src/commands/local.mjs index ec1b7ef3..c08aef73 100644 --- a/src/commands/local.mjs +++ b/src/commands/local.mjs @@ -1,5 +1,10 @@ +import chalk from "chalk"; +import { AbortError } from "fauna"; + +import { container } from "../cli.mjs"; import { ensureContainerRunning } from "../lib/docker-containers.mjs"; -import { CommandError } from "../lib/errors.mjs"; +import { CommandError, ValidationError } from "../lib/errors.mjs"; +import { colorize, Format } from "../lib/formatting/colorize.mjs"; /** * Starts the local Fauna container @@ -8,6 +13,7 @@ import { CommandError } from "../lib/errors.mjs"; * It will reject if the container is not ready after the maximum number of attempts. */ async function startLocal(argv) { + const color = argv.color; await ensureContainerRunning({ imageName: argv.image, containerName: argv.name, @@ -17,8 +23,67 @@ async function startLocal(argv) { pull: argv.pull, interval: argv.interval, maxAttempts: argv.maxAttempts, - color: argv.color, + color, }); + if (argv.database) { + await createDatabase(argv); + } +} + +async function createDatabase(argv) { + const { fql } = container.resolve("fauna"); + const { runQuery } = container.resolve("faunaClientV10"); + const logger = container.resolve("logger"); + const color = argv.color; + logger.stderr( + colorize(`[CreateDatabase] Creating database '${argv.database}'...`, { + format: Format.LOG, + color, + }), + ); + try { + const db = await runQuery({ + secret: "secret", + url: `http://${argv.hostIp}:${argv.hostPort}`, + query: fql` + let name = ${argv.database} + let database = Database.byName(name) + let protected = ${argv.protected ?? null} + let typechecked = ${argv.typechecked ?? null} + let priority = ${argv.priority ?? null} + if (database == null) { + Database.create({ + name: name, + protected: protected, + typechecked: typechecked, + priority: priority, + }) + } else if (protected == database.protected && typechecked == database.typechecked && priority == database.priority) { + database + } else { + abort(database) + }`, + options: { format: "decorated" }, + }); + logger.stderr( + colorize(`[CreateDatabase] Database '${argv.database}' created.`, { + format: Format.LOG, + color, + }), + ); + logger.stderr(colorize(db.data, { format: Format.FQL, color })); + } catch (e) { + if (e instanceof AbortError) { + throw new CommandError( + `${chalk.red(`[CreateDatabase] Database '${argv.database}' already exists but with differrent properties than requested:\n`)} +----------------- +${colorize(e.abort, { format: Format.FQL, color })} +----------------- +${chalk.red("Please use choose a different name using --name or align the --typechecked, --priority, and --protected with what is currently present.")}`, + ); + } + throw e; + } } /** @@ -67,17 +132,49 @@ function buildLocalCommand(yargs) { type: "boolean", default: true, }, + database: { + describe: + "The name of a database to create in the container. Omit to create no database.", + type: "string", + }, + typechecked: { + describe: + "Enable typechecking for the database. Valid only if --database is set.", + type: "boolean", + }, + protected: { + describe: + "Enable protected mode for the database. Protected mode disallows destructive schema changes. Valid only if --database is set.", + type: "boolean", + }, + priority: { + type: "number", + description: + "User-defined priority for the database. Valid only if --database is set.", + }, }) .check((argv) => { if (argv.maxAttempts < 1) { - throw new CommandError("--maxAttempts must be greater than 0.", { - hideHelp: false, - }); + throw new ValidationError("--maxAttempts must be greater than 0."); } if (argv.interval < 0) { - throw new CommandError( + throw new ValidationError( "--interval must be greater than or equal to 0.", - { hideHelp: false }, + ); + } + if (argv.typechecked && !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.", ); } return true; diff --git a/src/lib/docker-containers.mjs b/src/lib/docker-containers.mjs index a9f2ff98..22f301e2 100644 --- a/src/lib/docker-containers.mjs +++ b/src/lib/docker-containers.mjs @@ -154,7 +154,6 @@ async function findContainer({ containerName, hostPort }) { `[FindContainer] Container '${containerName}' is already \ in use on hostPort '${diffPort.PublicPort}'. Please use a new name via \ arguments --name --hostPort ${hostPort} to start the container.`, - { hideHelp: false }, ); } return result; @@ -212,7 +211,6 @@ async function createContainer({ throw new CommandError( `[StartContainer] The hostPort '${hostPort}' on IP '${hostIp}' is already occupied. \ Please pass a --hostPort other than '${hostPort}'.`, - { hideHelp: false }, ); } const dockerContainer = await docker.createContainer({ diff --git a/test/local.mjs b/test/local.mjs index 66c7abc4..fd5c037e 100644 --- a/test/local.mjs +++ b/test/local.mjs @@ -1,6 +1,7 @@ //@ts-check import { expect } from "chai"; +import { AbortError } from "fauna"; import sinon, { stub } from "sinon"; import { run } from "../src/cli.mjs"; @@ -78,6 +79,24 @@ describe("ensureContainerRunning", () => { net.createServer.returns(serverMock); }); + function setupCreateContainerMocks() { + docker.pull.onCall(0).resolves(); + docker.modem.followProgress.callsFake((stream, onFinished) => { + onFinished(); + }); + docker.listContainers.onCall(0).resolves([]); + fetch.onCall(0).resolves(f({})); // fast succeed the health check + logsStub.callsFake(async () => ({ + on: () => {}, + destroy: () => {}, + })); + docker.createContainer.resolves({ + start: startStub, + logs: logsStub, + unpause: unpauseStub, + }); + } + it("Shows a clear error to the user if something is already running on the desired port.", async () => { simulateError = true; docker.pull.onCall(0).resolves(); @@ -88,7 +107,6 @@ describe("ensureContainerRunning", () => { try { // Run the actual command await run("local --no-color", container); - throw new Error("Expected an error to be thrown."); } catch (_) { // Expected error, no action needed } @@ -100,26 +118,76 @@ describe("ensureContainerRunning", () => { "[StartContainer] The hostPort '8443' on IP '0.0.0.0' is already occupied. \ Please pass a --hostPort other than '8443'.", ); - expect(written).to.contain("fauna local"); + expect(written).not.to.contain("fauna local"); expect(written).not.to.contain("An unexpected"); }); - it("Creates and starts a container when none exists", async () => { - docker.pull.onCall(0).resolves(); - docker.modem.followProgress.callsFake((stream, onFinished) => { - onFinished(); + [ + "--database Foo", + "--database Foo --typechecked --protected --priority 1", + ].forEach((args) => { + it("Creates a database if requested", async () => { + setupCreateContainerMocks(); + const { runQuery } = container.resolve("faunaClientV10"); + runQuery.resolves({ + data: JSON.stringify({ name: "Foo" }, null, 2), + }); + await run(`local --no-color ${args}`, container); + expect(runQuery).to.have.been.calledWith({ + secret: "secret", + url: "http://0.0.0.0:8443", + query: sinon.match.any, + options: { format: "decorated" }, + }); + const written = stderrStream.getWritten(); + expect(written).to.contain("[CreateDatabase] Database 'Foo' created."); + expect(written).to.contain('"name": "Foo"'); }); - docker.listContainers.onCall(0).resolves([]); - fetch.onCall(0).resolves(f({})); // fast succeed the health check - logsStub.callsFake(async () => ({ - on: () => {}, - destroy: () => {}, - })); - docker.createContainer.resolves({ - start: startStub, - logs: logsStub, - unpause: unpauseStub, + }); + + it("Exits with an expected error if the create db query aborts", async () => { + setupCreateContainerMocks(); + const { runQuery } = container.resolve("faunaClientV10"); + runQuery.rejects(new AbortError({ error: { abort: "Taco" } })); + try { + await run(`local --no-color --database Foo`, container); + } catch (_) {} + expect(runQuery).to.have.been.calledWith({ + secret: "secret", + url: "http://0.0.0.0:8443", + query: sinon.match.any, + options: { format: "decorated" }, }); + const written = stderrStream.getWritten(); + expect(written).to.contain("Taco"); + expect(written).not.to.contain("fauna local"); + expect(written).not.to.contain("An unexpected"); + }); + + ["--typechecked", "--protected", "--priority 1"].forEach((args) => { + it("Rejects invalid create database args", async () => { + setupCreateContainerMocks(); + const { runQuery } = container.resolve("faunaClientV10"); + try { + await run(`local --no-color ${args}`, container); + } catch (_) {} + expect(runQuery).not.to.have.been.called; + const written = stderrStream.getWritten(); + expect(written).to.contain("fauna local"); + expect(written).not.to.contain("An unexpected"); + expect(written).to.contain("can only be set if"); + }); + }); + + it("Does not create a database when not requested to do so", async () => { + setupCreateContainerMocks(); + const { runQuery } = container.resolve("faunaClientV10"); + await run("local --no-color", container); + expect(runQuery).not.to.have.been.called; + }); + + it("Creates and starts a container when none exists", async () => { + setupCreateContainerMocks(); await run("local --no-color", container); expect(unpauseStub).not.to.have.been.called; expect(startStub).to.have.been.called; @@ -147,27 +215,10 @@ Please pass a --hostPort other than '8443'.", "8443/tcp": {}, }, }); - expect(logger.stderr).to.have.been.calledWith( - "[ContainerReady] Container 'faunadb' is up and healthy.", - ); }); it("The user can control the hostIp, hostPort, containerPort, and name", async () => { - docker.pull.onCall(0).resolves(); - docker.modem.followProgress.callsFake((stream, onFinished) => { - onFinished(); - }); - docker.listContainers.onCall(0).resolves([]); - fetch.onCall(0).resolves(f({})); // fast succeed the health check - logsStub.callsFake(async () => ({ - on: () => {}, - destroy: () => {}, - })); - docker.createContainer.resolves({ - start: startStub, - logs: logsStub, - unpause: unpauseStub, - }); + setupCreateContainerMocks(); await run( "local --no-color --hostPort 10 --containerPort 11 --name Taco --hostIp 127.0.0.1", container, @@ -193,17 +244,7 @@ Please pass a --hostPort other than '8443'.", }); it("Skips pull if --pull is false.", async () => { - docker.listContainers.onCall(0).resolves([]); - fetch.onCall(0).resolves(f({})); // fast succeed the health check - logsStub.callsFake(async () => ({ - on: () => {}, - destroy: () => {}, - })); - docker.createContainer.resolves({ - start: startStub, - logs: logsStub, - unpause: unpauseStub, - }); + setupCreateContainerMocks(); await run("local --no-color --pull false", container); expect(docker.pull).not.to.have.been.called; expect(docker.modem.followProgress).not.to.have.been.called; @@ -216,21 +257,10 @@ Please pass a --hostPort other than '8443'.", }); it("Fails start with a prompt to contact Fauna if pull fails.", async () => { + setupCreateContainerMocks(); docker.pull.onCall(0).rejects(new Error("Remote repository not found")); - docker.listContainers.onCall(0).resolves([]); - fetch.onCall(0).resolves(f({})); // fast succeed the health check - logsStub.callsFake(async () => ({ - on: () => {}, - destroy: () => {}, - })); - docker.createContainer.resolves({ - start: startStub, - logs: logsStub, - unpause: unpauseStub, - }); try { await run("local --no-color", container); - throw new Error("Expected an error to be thrown."); } catch (_) {} expect(docker.pull).to.have.been.called; expect(docker.modem.followProgress).not.to.have.been.called; @@ -248,31 +278,11 @@ https://support.fauna.com/hc/en-us/requests/new`, }); it("Throws an error if the health check fails", async () => { - docker.pull.onCall(0).resolves(); - docker.modem.followProgress.callsFake((stream, onFinished) => { - onFinished(); - }); - docker.listContainers.onCall(0).resolves([ - { - State: "created", - Names: ["/faunadb"], - Ports: [{ PublicPort: 8443 }], - }, - ]); - logsStub.callsFake(async () => ({ - on: () => {}, - destroy: () => {}, - })); - docker.getContainer.onCall(0).returns({ - logs: logsStub, - start: startStub, - unpause: unpauseStub, - }); + setupCreateContainerMocks(); fetch.onCall(0).rejects(); fetch.resolves(f({}, 503)); // fail from http try { await run("local --no-color --interval 0 --maxAttempts 3", container); - throw new Error("Expected an error to be thrown."); } catch (_) {} const written = stderrStream.getWritten(); expect(written).to.contain("with HTTP status: '503'"); @@ -308,7 +318,6 @@ https://support.fauna.com/hc/en-us/requests/new`, }); try { await run("local --no-color", container); - throw new Error("Expected an error to be thrown."); } catch (_) {} const written = stderrStream.getWritten(); expect(written).to.contain( @@ -320,7 +329,6 @@ https://support.fauna.com/hc/en-us/requests/new`, it("throws an error if interval is less than 0", async () => { try { await run("local --no-color --interval -1", container); - throw new Error("Expected an error to be thrown."); } catch (_) {} const written = stderrStream.getWritten(); expect(written).to.contain( @@ -333,7 +341,6 @@ https://support.fauna.com/hc/en-us/requests/new`, it("throws an error if maxAttempts is less than 1", async () => { try { await run("local --no-color --maxAttempts 0", container); - throw new Error("Expected an error to be thrown."); } catch (_) {} const written = stderrStream.getWritten(); expect(written).to.contain("--maxAttempts must be greater than 0."); @@ -473,7 +480,6 @@ https://support.fauna.com/hc/en-us/requests/new`, try { await run(`local --hostPort ${desiredPort}`, container); - throw new Error("Expected an error to be thrown."); } catch (_) {} expect(docker.listContainers).to.have.been.calledWith({ all: true, @@ -489,6 +495,6 @@ Please use a new name via arguments --name --hostPort ${desiredPort} \ to start the container.`, ); expect(written).not.to.contain("An unexpected"); - expect(written).to.contain("fauna local"); // help text + expect(written).not.to.contain("fauna local"); // help text }); }); From aa22facc936c63ec02445f926175824ae0979530 Mon Sep 17 00:00:00 2001 From: "E. Cooper" Date: Mon, 16 Dec 2024 08:56:51 -0800 Subject: [PATCH 3/3] Clean up error handling to have less unexpected errors (#529) * Clean up error handling to have less unexpected errors * Update src/commands/database/list.mjs Fix lint --------- Co-authored-by: Matthew Wilde --- src/commands/database/create.mjs | 45 +++++---- src/commands/database/delete.mjs | 19 ++-- src/commands/database/list.mjs | 9 +- src/lib/account.mjs | 72 ++++++++++---- src/lib/auth/accountKeys.mjs | 5 +- src/lib/db.mjs | 13 ++- src/lib/errors.mjs | 69 +++++++++++++- src/lib/fauna-account-client.mjs | 6 +- src/lib/fauna-client.mjs | 20 ++-- src/lib/fauna.mjs | 86 +++++------------ src/lib/faunadb.mjs | 40 +++++++- src/lib/misc.mjs | 16 ---- test/database/create.mjs | 12 +-- test/database/delete.mjs | 10 +- test/database/list.mjs | 4 +- test/lib/account.mjs | 159 +++++++++++++++++++++++++++++++ test/lib/fauna.mjs | 120 +++++++++++++++++++++++ test/lib/faunadb.mjs | 99 +++++++++++++++++++ test/schema/schema.mjs | 40 +++++++- test/shell.mjs | 1 + 20 files changed, 669 insertions(+), 176 deletions(-) create mode 100644 test/lib/account.mjs create mode 100644 test/lib/fauna.mjs create mode 100644 test/lib/faunadb.mjs diff --git a/src/commands/database/create.mjs b/src/commands/database/create.mjs index 1a2d5762..db1691d7 100644 --- a/src/commands/database/create.mjs +++ b/src/commands/database/create.mjs @@ -1,10 +1,10 @@ //@ts-check - -import { FaunaError } from "fauna"; +import { ServiceError } from "fauna"; import { container } from "../../cli.mjs"; import { validateDatabaseOrSecret } from "../../lib/command-helpers.mjs"; -import { throwForError } from "../../lib/fauna.mjs"; +import { CommandError } from "../../lib/errors.mjs"; +import { faunaToCommandError } from "../../lib/fauna.mjs"; import { getSecret, retryInvalidCredsOnce } from "../../lib/fauna-client.mjs"; import { colorize, Format } from "../../lib/formatting/colorize.mjs"; @@ -44,26 +44,29 @@ async function createDatabase(argv) { logger.stdout(argv.name); } } catch (e) { - if (e instanceof FaunaError) { - throwForError(e, { - onConstraintFailure: (err) => { - const cf = err.constraint_failures; - if (cf && cf.length > 0) { - const nameIsInvalidIdentifier = cf.some( - (failure) => - failure?.paths?.length === 1 && - failure?.paths?.[0]?.[0] === "name" && - failure?.message === "Invalid identifier.", + faunaToCommandError(e, (err) => { + if (err instanceof ServiceError && err.code === "constraint_failure") { + const cf = err.constraint_failures; + if (cf && cf.length > 0) { + const nameIsInvalidIdentifier = cf.some( + (failure) => + failure?.paths?.length === 1 && + failure?.paths?.[0]?.[0] === "name" && + failure?.message === "Invalid identifier.", + ); + if (nameIsInvalidIdentifier) { + throw new CommandError( + `The database name '${argv.name}' is invalid. Database names must begin with letters and include only letters, numbers, and underscores.`, + { cause: err }, ); - if (nameIsInvalidIdentifier) { - return `Constraint failure: The database name '${argv.name}' is invalid. Database names must begin with letters and include only letters, numbers, and underscores.`; - } } - return `Constraint failure: The database '${argv.name}' already exists or one of the provided options is invalid.`; - }, - }); - } - throw e; + } + throw new CommandError( + `The database '${argv.name}' already exists or one of the provided options is invalid.`, + { cause: err }, + ); + } + }); } } diff --git a/src/commands/database/delete.mjs b/src/commands/database/delete.mjs index 86bab93a..17b7336b 100644 --- a/src/commands/database/delete.mjs +++ b/src/commands/database/delete.mjs @@ -1,10 +1,11 @@ //@ts-check -import { FaunaError } from "fauna"; +import { ServiceError } from "fauna"; import { container } from "../../cli.mjs"; import { validateDatabaseOrSecret } from "../../lib/command-helpers.mjs"; -import { throwForError } from "../../lib/fauna.mjs"; +import { CommandError } from "../../lib/errors.mjs"; +import { faunaToCommandError } from "../../lib/fauna.mjs"; import { getSecret, retryInvalidCredsOnce } from "../../lib/fauna-client.mjs"; async function runDeleteQuery(secret, argv) { @@ -29,13 +30,13 @@ async function deleteDatabase(argv) { // We use stderr for messaging and there's no stdout output for a deleted database logger.stderr(`Database '${argv.name}' was successfully deleted.`); } catch (e) { - if (e instanceof FaunaError) { - throwForError(e, { - onDocumentNotFound: () => - `Not found: Database '${argv.name}' not found. Please check the database name and try again.`, - }); - } - throw e; + faunaToCommandError(e, (err) => { + if (err instanceof ServiceError && err.code === "document_not_found") { + throw new CommandError( + `Database '${argv.name}' not found. Please check the database name and try again.`, + ); + } + }); } } diff --git a/src/commands/database/list.mjs b/src/commands/database/list.mjs index e5c78ac5..8fb27e7c 100644 --- a/src/commands/database/list.mjs +++ b/src/commands/database/list.mjs @@ -1,9 +1,7 @@ //@ts-check -import { FaunaError } from "fauna"; - import { container } from "../../cli.mjs"; -import { throwForError } from "../../lib/fauna.mjs"; +import { faunaToCommandError } from "../../lib/fauna.mjs"; import { FaunaAccountClient } from "../../lib/fauna-account-client.mjs"; import { colorize, Format } from "../../lib/formatting/colorize.mjs"; @@ -50,10 +48,7 @@ async function listDatabasesWithSecret(argv) { expression: `Database.all().paginate(${pageSize}).data { ${getOutputFields(argv)} }`, }); } catch (e) { - if (e instanceof FaunaError) { - throwForError(e); - } - throw e; + return faunaToCommandError(e); } } diff --git a/src/lib/account.mjs b/src/lib/account.mjs index 536625e9..5caee165 100644 --- a/src/lib/account.mjs +++ b/src/lib/account.mjs @@ -1,6 +1,9 @@ import { container } from "../cli.mjs"; -import { ValidationError } from "./errors.mjs"; -import { InvalidCredsError, UnauthorizedError } from "./misc.mjs"; +import { + AuthenticationError, + AuthorizationError, + CommandError, +} from "./errors.mjs"; /** * * @param {Object} opts @@ -58,36 +61,65 @@ export async function makeAccountRequest({ return parseResponse(response, shouldThrow); } +/** + * Throws an error based on the status code of the response + * + * @param {Response} response + * @param {boolean} responseIsJSON + * @throws {AuthenticationError | AuthorizationError | CommandError | Error} + */ +const accountToCommandError = async (response, responseIsJSON) => { + let message = `Failed to make request to Fauna account API [${response.status}]`; + + let { code, reason, body } = {}; + if (responseIsJSON) { + body = await response.json(); + ({ reason, code } = body); + message += `: ${code} - ${reason}`; + } + + // If consumers want to do more with this, they analyze the cause + const responseAsCause = new Error(message); + responseAsCause.status = response.status; + responseAsCause.body = body; + responseAsCause.headers = response.headers; + responseAsCause.code = code; + responseAsCause.reason = reason; + + switch (response.status) { + case 401: + throw new AuthenticationError({ cause: responseAsCause }); + case 403: + throw new AuthorizationError({ cause: responseAsCause }); + case 400: + case 404: + throw new CommandError(reason ?? message, { + cause: responseAsCause, + hideHelp: true, + }); + default: + throw new Error(message, { cause: responseAsCause }); + } +}; + /** * Returns the proper result based on the content type of the account API response * Conditionally throws errors for status codes > 400 * * @param {Response} response result of the fetch call to account api * @param {boolean} shouldThrow whether to ignore an error from the result - * @returns + * @returns {Promise} - The response from the request + * @throws {AuthenticationError | AuthorizationError | CommandError | Error} */ -async function parseResponse(response, shouldThrow) { +export async function parseResponse(response, shouldThrow) { const responseType = response?.headers?.get("content-type") || "application/json"; const responseIsJSON = responseType.includes("application/json"); + if (response.status >= 400 && shouldThrow) { - let message = `Failed to make request to Fauna account API [${response.status}]`; - if (responseIsJSON) { - const body = await response.json(); - const { reason, code } = body; - message += `: ${code} - ${reason}`; - } - switch (response.status) { - case 401: - throw new InvalidCredsError(message); - case 403: - throw new UnauthorizedError(message); - case 404: - throw new ValidationError(message); - default: - throw new Error(message); - } + await accountToCommandError(response, responseIsJSON); } + const result = responseIsJSON ? await response.json() : await response; return result; } diff --git a/src/lib/auth/accountKeys.mjs b/src/lib/auth/accountKeys.mjs index 2f145deb..815147c7 100644 --- a/src/lib/auth/accountKeys.mjs +++ b/src/lib/auth/accountKeys.mjs @@ -1,8 +1,7 @@ import { container } from "../../cli.mjs"; -import { CommandError } from "../errors.mjs"; +import { AuthenticationError, CommandError } from "../errors.mjs"; import { FaunaAccountClient } from "../fauna-account-client.mjs"; import { AccountKeyStorage } from "../file-util.mjs"; -import { InvalidCredsError } from "../misc.mjs"; /** * Class representing the account key(s) available to the user. @@ -118,7 +117,7 @@ export class AccountKeys { const databaseKeys = container.resolve("credentials").databaseKeys; databaseKeys.updateAccountKey(newAccountKey.accountKey); } catch (e) { - if (e instanceof InvalidCredsError) { + if (e instanceof AuthenticationError) { this.promptLogin(); } else { throw e; diff --git a/src/lib/db.mjs b/src/lib/db.mjs index 069abe57..1814fe6e 100644 --- a/src/lib/db.mjs +++ b/src/lib/db.mjs @@ -1,7 +1,11 @@ //@ts-check import { container } from "../cli.mjs"; -import { CommandError, NETWORK_ERROR_MESSAGE } from "./errors.mjs"; +import { + AuthenticationError, + CommandError, + NETWORK_ERROR_MESSAGE, +} from "./errors.mjs"; import { retryInvalidCredsOnce } from "./fauna-client.mjs"; function buildParamsString({ argv, params, path }) { @@ -75,13 +79,12 @@ export async function makeFaunaRequest({ const obj = await response.json(); if (obj.error && shouldThrow) { - const err = new CommandError(obj.error.message); - err.name = obj.error.code; - if (obj.error.code === "unauthorized") { - err.message = "The database secret provided is invalid."; + throw new AuthenticationError({ cause: obj.error }); } + const err = new CommandError(obj.error.message, { cause: obj.error }); + err.name = obj.error.code; throw err; } diff --git a/src/lib/errors.mjs b/src/lib/errors.mjs index 877a758e..1111ea05 100644 --- a/src/lib/errors.mjs +++ b/src/lib/errors.mjs @@ -8,6 +8,10 @@ const BUG_REPORT_MESSAGE = "If you believe this is a bug, please report this issue on GitHub: https://github.com/fauna/fauna-shell/issues"; export const SUPPORT_MESSAGE = "If this issue persists contact support: https://support.fauna.com/hc/en-us/requests/new"; +export const AUTHENTICATION_ERROR_MESSAGE = + "Authentication failed. Log in using 'fauna login' or provide a valid database secret with --secret."; +export const AUTHORIZATION_ERROR_MESSAGE = + "Authorization failed. The current user or secret does not have the required permissions to complete this action."; // This error message is used in a few places where we handle network errors. export const NETWORK_ERROR_MESSAGE = @@ -71,6 +75,63 @@ export class ValidationError extends CommandError { } } +/** + * An error that is thrown when the user provides invalid credentials. + */ +export class AuthenticationError extends CommandError { + /** + * @param {string | opts} [messageOrOpts] + * @param {object} [opts] + * @param {number} [opts.exitCode] + * @param {boolean} [opts.hideHelp] + * @param {Error} [opts.cause] + */ + constructor(messageOrOpts = AUTHENTICATION_ERROR_MESSAGE, opts = {}) { + let message = AUTHENTICATION_ERROR_MESSAGE; + let resolvedOpts = { + exitCode: 1, + hideHelp: true, + cause: undefined, + ...opts, + }; + + if (typeof messageOrOpts === "string") { + message = messageOrOpts; + } else { + resolvedOpts = { ...resolvedOpts, ...messageOrOpts }; + } + + super(message, resolvedOpts); + this.name = "AuthenticationError"; + } +} + +export class AuthorizationError extends CommandError { + /** + * @param {string | opts} [messageOrOpts] + * @param {object} [opts] + * @param {number} [opts.exitCode] + * @param {boolean} [opts.hideHelp] + * @param {Error} [opts.cause] + */ + constructor(messageOrOpts = AUTHORIZATION_ERROR_MESSAGE, opts = {}) { + let message = AUTHORIZATION_ERROR_MESSAGE; + let resolvedOpts = { + exitCode: 1, + hideHelp: true, + cause: undefined, + ...opts, + }; + if (typeof messageOrOpts === "string") { + message = messageOrOpts; + } else { + resolvedOpts = { ...resolvedOpts, ...messageOrOpts }; + } + super(message, resolvedOpts); + this.name = "AuthorizationError"; + } +} + /** * Returns true if the error is an error potentially thrown by yargs * @param {Error} error @@ -99,7 +160,13 @@ function isYargsError(error) { * @returns {boolean} */ export function isUnknownError(error) { - return !isYargsError(error) && !(error instanceof CommandError); + return ( + !isYargsError(error) && + !(error instanceof CommandError) && + !(error instanceof ValidationError) && + !(error instanceof AuthorizationError) && + !(error instanceof AuthenticationError) + ); } export const handleParseYargsError = async ( diff --git a/src/lib/fauna-account-client.mjs b/src/lib/fauna-account-client.mjs index e8566859..efea7696 100644 --- a/src/lib/fauna-account-client.mjs +++ b/src/lib/fauna-account-client.mjs @@ -1,7 +1,7 @@ //@ts-check import { container } from "../cli.mjs"; -import { InvalidCredsError } from "./misc.mjs"; +import { AuthenticationError } from "./errors.mjs"; // const KEY_TTL_DEFAULT_MS = 1000 * 60 * 60 * 24; @@ -20,7 +20,7 @@ export class FaunaAccountClient { try { result = await original(await this.getRequestArgs(args)); } catch (e) { - if (e instanceof InvalidCredsError) { + if (e instanceof AuthenticationError) { try { logger.debug( "401 in account api, attempting to refresh session", @@ -31,7 +31,7 @@ export class FaunaAccountClient { const updatedArgs = await this.getRequestArgs(args); result = await original(updatedArgs); } catch (e) { - if (e instanceof InvalidCredsError) { + if (e instanceof AuthenticationError) { logger.debug( "Failed to refresh session, expired or missing refresh token", "creds", diff --git a/src/lib/fauna-client.mjs b/src/lib/fauna-client.mjs index 69949e68..bd8f95f8 100644 --- a/src/lib/fauna-client.mjs +++ b/src/lib/fauna-client.mjs @@ -1,7 +1,9 @@ //@ts-check import { container } from "../cli.mjs"; -import { isUnknownError, ValidationError } from "./errors.mjs"; +import { isUnknownError } from "./errors.mjs"; +import { faunaToCommandError } from "./fauna.mjs"; +import { faunadbToCommandError } from "./faunadb.mjs"; import { colorize, Format } from "./formatting/colorize.mjs"; const SUMMARY_FQL_REGEX = /^(\s\s\|)|(\d\s\|)/; @@ -117,19 +119,17 @@ export const isQueryable = async (argv) => { try { await runQueryFromString("1+1", argv); } catch (err) { + // Three things can throw errors here. Stuff we know, + // like authx, v10 errors, and v4 errors or stuff we don't know. if (!isUnknownError(err)) { throw err; } - throw new ValidationError( - formatError(err, { - apiVersion: argv.apiVersion, - color: false, - }), - { - cause: err, - }, - ); + if (argv.apiVersion === "4") { + faunadbToCommandError(err); + } else { + faunaToCommandError(err); + } } return true; diff --git a/src/lib/fauna.mjs b/src/lib/fauna.mjs index 93b708f1..a819c666 100644 --- a/src/lib/fauna.mjs +++ b/src/lib/fauna.mjs @@ -4,16 +4,16 @@ */ import chalk from "chalk"; -import { - ClientClosedError, - ClientError, - NetworkError, - ProtocolError, - ServiceError, -} from "fauna"; +import { NetworkError, ServiceError } from "fauna"; import { container } from "../cli.mjs"; -import { NETWORK_ERROR_MESSAGE, ValidationError } from "./errors.mjs"; +import { + AuthenticationError, + AuthorizationError, + CommandError, + NETWORK_ERROR_MESSAGE, + ValidationError, +} from "./errors.mjs"; import { formatQuerySummary } from "./fauna-client.mjs"; import { colorize, Format } from "./formatting/colorize.mjs"; @@ -170,66 +170,32 @@ export const formatQueryResponse = (res, opts = {}) => { * commands on the users behalf and want to provide a more helpful error message. * * @param {import("fauna").FaunaError} e - The Fauna error to handle - * @param {object} [handlers] - Optional error handlers - * @param {(e: ServiceError) => string} [handlers.onInvalidQuery] - Handler for invalid query errors - * @param {(e: ServiceError) => string} [handlers.onInvalidRequest] - Handler for invalid request errors - * @param {(e: ServiceError) => string} [handlers.onAbort] - Handler for aborted operation errors - * @param {(e: ServiceError) => string} [handlers.onConstraintFailure] - Handler for constraint violation errors - * @param {(e: ServiceError) => string} [handlers.onUnauthorized] - Handler for unauthorized access errors - * @param {(e: ServiceError) => string} [handlers.onForbidden] - Handler for forbidden access errors - * @param {(e: ServiceError) => string} [handlers.onContendedTransaction] - Handler for transaction contention errors - * @param {(e: ServiceError) => string} [handlers.onLimitExceeded] - Handler for rate/resource limit errors - * @param {(e: ServiceError) => string} [handlers.onTimeOut] - Handler for timeout errors - * @param {(e: ServiceError) => string} [handlers.onInternalError] - Handler for internal server errors - * @param {(e: ServiceError) => string} [handlers.onDocumentNotFound] - Handler for document not found errors - * @param {(e: ClientError) => string} [handlers.onClientError] - Handler for general client errors - * @param {(e: ClientClosedError) => string} [handlers.onClientClosedError] - Handler for closed client errors - * @param {(e: NetworkError) => string} [handlers.onNetworkError] - Handler for network-related errors - * @param {(e: ProtocolError) => string} [handlers.onProtocolError] - Handler for protocol-related errors + * @param {(e: import("fauna").FaunaError) => void} [handler] - Optional error handler to handle and throw in * @throws {Error} Always throws an error with a message based on the error code or handler response * @returns {never} This function always throws an error */ -// eslint-disable-next-line complexity -export const throwForError = (e, handlers = {}) => { + +export const faunaToCommandError = (e, handler) => { + if (handler) { + handler(e); + } + if (e instanceof ServiceError) { switch (e.code) { - case "invalid_query": - throw new Error(handlers.onInvalidQuery?.(e) ?? formatError(e)); - case "invalid_request ": - throw new Error(handlers.onInvalidRequest?.(e) ?? formatError(e)); - case "abort": - throw new Error(handlers.onAbort?.(e) ?? formatError(e)); - case "constraint_failure": - throw new Error(handlers.onConstraintFailure?.(e) ?? formatError(e)); case "unauthorized": - throw new Error( - handlers.onUnauthorized?.(e) ?? - "Authentication failed: Please either log in using 'fauna login' or provide a valid database secret with '--secret'.", - ); + throw new AuthenticationError({ cause: e }); case "forbidden": - throw new Error(handlers.onForbidden?.(e) ?? formatError(e)); - case "contended_transaction": - throw new Error(handlers.onContendedTransaction?.(e) ?? formatError(e)); - case "limit_exceeded": - throw new Error(handlers.onLimitExceeded?.(e) ?? formatError(e)); - case "time_out": - throw new Error(handlers.onTimeOut?.(e) ?? formatError(e)); - case "internal_error": - throw new Error(handlers.onInternalError?.(e) ?? formatError(e)); - case "document_not_found": - throw new Error(handlers.onDocumentNotFound?.(e) ?? formatError(e)); + throw new AuthorizationError({ cause: e }); + case "permission_denied": + throw new AuthorizationError({ cause: e }); default: - throw e; + throw new CommandError(formatError(e), { cause: e }); } - } else if (e instanceof ClientError) { - throw new Error(handlers.onClientError?.(e) ?? formatError(e)); - } else if (e instanceof ClientClosedError) { - throw new Error(handlers.onClientClosedError?.(e) ?? formatError(e)); - } else if (e instanceof NetworkError) { - throw new Error(handlers.onNetworkError?.(e) ?? formatError(e)); - } else if (e instanceof ProtocolError) { - throw new Error(handlers.onProtocolError?.(e) ?? formatError(e)); - } else { - throw e; } + + if (e instanceof NetworkError) { + throw new CommandError(NETWORK_ERROR_MESSAGE, { cause: e }); + } + + throw e; }; diff --git a/src/lib/faunadb.mjs b/src/lib/faunadb.mjs index 84418738..353ca1f4 100644 --- a/src/lib/faunadb.mjs +++ b/src/lib/faunadb.mjs @@ -1,8 +1,15 @@ // @ts-check import { createContext, runInContext } from "node:vm"; +import faunadb from "faunadb"; + import { container } from "../cli.mjs"; -import { NETWORK_ERROR_MESSAGE } from "./errors.mjs"; +import { + AuthenticationError, + AuthorizationError, + CommandError, + NETWORK_ERROR_MESSAGE, +} from "./errors.mjs"; import { colorize, Format } from "./formatting/colorize.mjs"; /** @@ -125,6 +132,37 @@ export const formatError = (err, opts = {}) => { return colorize(errorPrefix + err.message, { color }); }; +/** + * Converts a Fauna HTTP error to a CommandError. + * @param {any} err - The error to convert + * @param {(e: import("fauna").FaunaError) => void} [handler] - Optional error handler to handle and throw in + */ +export const faunadbToCommandError = (err, handler) => { + if (handler) { + handler(err); + } + + if (err instanceof faunadb.errors.FaunaHTTPError) { + switch (err.name) { + case "Unauthorized": + throw new AuthenticationError({ cause: err }); + case "PermissionDenied": + throw new AuthorizationError({ cause: err }); + case "BadRequest": + case "NotFound": + throw new CommandError(formatError(err, { raw: true }), { cause: err }); + default: + throw err; + } + } + + if (err.name === "TypeError" && err.message.includes("fetch failed")) { + throw new CommandError(NETWORK_ERROR_MESSAGE, { cause: err }); + } + + throw err; +}; + /** * Formats a V4 Fauna query response. * @param {any} res - The query response to format diff --git a/src/lib/misc.mjs b/src/lib/misc.mjs index f60d7d5d..73fd4f1c 100644 --- a/src/lib/misc.mjs +++ b/src/lib/misc.mjs @@ -1,19 +1,3 @@ -export class InvalidCredsError extends Error { - constructor(message) { - super(message); - this.name = "InvalidCredsError"; - this.status = 401; - } -} - -export class UnauthorizedError extends Error { - constructor(message) { - super(message); - this.name = "UnauthorizedError"; - this.status = 403; - } -} - export function isTTY() { return process.stdout.isTTY; } diff --git a/test/database/create.mjs b/test/database/create.mjs index f16dd411..c997c57c 100644 --- a/test/database/create.mjs +++ b/test/database/create.mjs @@ -6,6 +6,7 @@ import sinon from "sinon"; import { run } from "../../src/cli.mjs"; import { setupTestContainer as setupContainer } from "../../src/config/setup-test-container.mjs"; +import { AUTHENTICATION_ERROR_MESSAGE } from "../../src/lib/errors.mjs"; import { mockAccessKeysFile } from "../helpers.mjs"; describe("database create", () => { @@ -50,7 +51,7 @@ describe("database create", () => { error: { code: "constraint_failure", message: "whatever" }, }), expectedMessage: - "Constraint failure: The database 'testdb' already exists or one of the provided options is invalid.", + "The database 'testdb' already exists or one of the provided options is invalid.", }, { error: new ServiceError({ @@ -66,14 +67,13 @@ describe("database create", () => { }, }), expectedMessage: - "Constraint failure: The database name 'testdb' is invalid. Database names must begin with letters and include only letters, numbers, and underscores.", + "The database name 'testdb' is invalid. Database names must begin with letters and include only letters, numbers, and underscores.", }, { error: new ServiceError({ error: { code: "unauthorized", message: "whatever" }, }), - expectedMessage: - "Authentication failed: Please either log in using 'fauna login' or provide a valid database secret with '--secret'.", + expectedMessage: AUTHENTICATION_ERROR_MESSAGE, }, ].forEach(({ error, expectedMessage }) => { it(`handles ${error.code} errors when calling fauna`, async () => { @@ -155,9 +155,7 @@ describe("database create", () => { expect(makeAccountRequest).to.not.have.been.called; expect(logger.stderr).to.have.been.calledWith( - sinon.match( - "Authentication failed: Please either log in using 'fauna login' or provide a valid database secret with '--secret'.", - ), + sinon.match(AUTHENTICATION_ERROR_MESSAGE), ); }); }); diff --git a/test/database/delete.mjs b/test/database/delete.mjs index 2d715867..712b73ca 100644 --- a/test/database/delete.mjs +++ b/test/database/delete.mjs @@ -6,6 +6,7 @@ import sinon from "sinon"; import { run } from "../../src/cli.mjs"; import { setupTestContainer as setupContainer } from "../../src/config/setup-test-container.mjs"; +import { AUTHENTICATION_ERROR_MESSAGE } from "../../src/lib/errors.mjs"; import { mockAccessKeysFile } from "../helpers.mjs"; describe("database delete", () => { @@ -49,15 +50,14 @@ describe("database delete", () => { error: new ServiceError({ error: { code: "unauthorized", message: "whatever" }, }), - expectedMessage: - "Authentication failed: Please either log in using 'fauna login' or provide a valid database secret with '--secret'.", + expectedMessage: AUTHENTICATION_ERROR_MESSAGE, }, { error: new ServiceError({ error: { code: "document_not_found", message: "whatever" }, }), expectedMessage: - "Not found: Database 'testdb' not found. Please check the database name and try again.", + "Database 'testdb' not found. Please check the database name and try again.", }, ].forEach(({ error, expectedMessage }) => { it(`handles ${error.code} errors when calling fauna`, async () => { @@ -120,9 +120,7 @@ describe("database delete", () => { expect(makeAccountRequest).to.not.have.been.called; expect(logger.stderr).to.have.been.calledWith( - sinon.match( - "Authentication failed: Please either log in using 'fauna login' or provide a valid database secret with '--secret'.", - ), + sinon.match(AUTHENTICATION_ERROR_MESSAGE), ); }); }); diff --git a/test/database/list.mjs b/test/database/list.mjs index 90e8e315..614bd06b 100644 --- a/test/database/list.mjs +++ b/test/database/list.mjs @@ -6,6 +6,7 @@ import sinon from "sinon"; import { run } from "../../src/cli.mjs"; import { setupTestContainer as setupContainer } from "../../src/config/setup-test-container.mjs"; +import { AUTHENTICATION_ERROR_MESSAGE } from "../../src/lib/errors.mjs"; import { colorize } from "../../src/lib/formatting/colorize.mjs"; import { mockAccessKeysFile } from "../helpers.mjs"; @@ -126,8 +127,7 @@ describe("database list", () => { error: new ServiceError({ error: { code: "unauthorized", message: "whatever" }, }), - expectedMessage: - "Authentication failed: Please either log in using 'fauna login' or provide a valid database secret with '--secret'.", + expectedMessage: AUTHENTICATION_ERROR_MESSAGE, }, ].forEach(({ error, expectedMessage }) => { it(`handles ${error.code} errors when calling fauna`, async () => { diff --git a/test/lib/account.mjs b/test/lib/account.mjs new file mode 100644 index 00000000..c887be7e --- /dev/null +++ b/test/lib/account.mjs @@ -0,0 +1,159 @@ +import { expect } from "chai"; + +import { parseResponse } from "../../src/lib/account.mjs"; +import { + AuthenticationError, + AuthorizationError, + CommandError, +} from "../../src/lib/errors.mjs"; + +describe("parseResponse", () => { + const createMockResponse = ( + status, + body = {}, + contentType = "application/json", + ) => { + return { + status, + headers: { + get: () => contentType, + }, + json: async () => body, + }; + }; + + it("should throw AuthenticationError for 401 status", async () => { + const response = createMockResponse(401, { + code: "unauthorized", + reason: "Invalid credentials", + }); + + try { + await parseResponse(response, true); + } catch (error) { + expect(error).to.be.instanceOf(AuthenticationError); + } + }); + + it("should throw AuthorizationError for 403 status", async () => { + const response = createMockResponse(403, { + code: "permission_denied", + reason: "Insufficient permissions", + }); + + try { + await parseResponse(response, true); + } catch (error) { + expect(error).to.be.instanceOf(AuthorizationError); + } + }); + + it("should throw CommandError for 400 status", async () => { + const response = createMockResponse(400, { + code: "bad_request", + reason: "Invalid parameters", + }); + + try { + await parseResponse(response, true); + } catch (error) { + expect(error).to.be.instanceOf(CommandError); + } + }); + + it("should throw CommandError for 404 status", async () => { + const response = createMockResponse(404, { + code: "not_found", + reason: "Resource not found", + }); + + try { + await parseResponse(response, true); + } catch (error) { + expect(error).to.be.instanceOf(CommandError); + } + }); + + it("should throw generic Error for other error status codes", async () => { + const response = createMockResponse(500, { + code: "internal_error", + reason: "Server error", + }); + + try { + await parseResponse(response, true); + } catch (error) { + expect(error).to.be.instanceOf(Error); + } + }); + + it("should include status code in error message", async () => { + const response = createMockResponse(500, { + code: "internal_error", + reason: "Server error", + }); + + try { + await parseResponse(response, true); + } catch (error) { + expect(error.message).to.include("[500]"); + expect(error.message).to.include("internal_error"); + expect(error.message).to.include("Server error"); + } + }); + + it("should not throw error when shouldThrow is false", async () => { + const response = createMockResponse(400, { + code: "bad_request", + reason: "Invalid parameters", + }); + + const result = await parseResponse(response, false); + expect(result).to.deep.equal({ + code: "bad_request", + reason: "Invalid parameters", + }); + }); + + it("should handle non-JSON responses", async () => { + const response = { + status: 400, + headers: { + get: () => "text/plain", + }, + }; + + try { + await parseResponse(response, true); + } catch (error) { + expect(error).to.be.instanceOf(CommandError); + expect(error.message).to.include("[400]"); + } + }); + + it("should preserve error details in cause", async () => { + const responseBody = { + code: "bad_request", + reason: "Invalid parameters", + }; + const response = createMockResponse(400, responseBody); + + try { + await parseResponse(response, true); + } catch (error) { + expect(error.cause).to.exist; + expect(error.cause.status).to.equal(400); + expect(error.cause.body).to.deep.equal(responseBody); + expect(error.cause.code).to.equal("bad_request"); + expect(error.cause.reason).to.equal("Invalid parameters"); + } + }); + + it("should return parsed JSON for successful responses", async () => { + const responseBody = { data: "success" }; + const response = createMockResponse(200, responseBody); + + const result = await parseResponse(response, true); + expect(result).to.deep.equal(responseBody); + }); +}); diff --git a/test/lib/fauna.mjs b/test/lib/fauna.mjs new file mode 100644 index 00000000..cf5aeb8e --- /dev/null +++ b/test/lib/fauna.mjs @@ -0,0 +1,120 @@ +import { expect } from "chai"; +import { NetworkError, ServiceError } from "fauna"; + +import { + AuthenticationError, + AuthorizationError, + CommandError, + NETWORK_ERROR_MESSAGE, +} from "../../src/lib/errors.mjs"; +import { faunaToCommandError } from "../../src/lib/fauna.mjs"; + +describe("faunaToCommandError", () => { + it("should convert unauthorized ServiceError to AuthenticationError", () => { + const serviceError = new ServiceError({ + error: { + code: "unauthorized", + message: "Invalid token", + }, + }); + + try { + faunaToCommandError(serviceError); + } catch (error) { + expect(error).to.be.instanceOf(AuthenticationError); + expect(error.cause).to.equal(serviceError); + } + }); + + it("should convert forbidden ServiceError to AuthorizationError", () => { + const serviceError = new ServiceError({ + error: { + code: "forbidden", + message: "Permission denied", + }, + }); + + try { + faunaToCommandError(serviceError); + } catch (error) { + expect(error).to.be.instanceOf(AuthorizationError); + expect(error.cause).to.equal(serviceError); + } + }); + + it("should convert permission_denied ServiceError to AuthorizationError", () => { + const serviceError = new ServiceError({ + error: { + code: "permission_denied", + message: "No permission", + }, + }); + + try { + faunaToCommandError(serviceError); + } catch (error) { + expect(error).to.be.instanceOf(AuthorizationError); + expect(error.cause).to.equal(serviceError); + } + }); + + it("should convert other ServiceErrors to CommandError", () => { + const serviceError = new ServiceError({ + error: { + code: "internal_error", + message: "Unknown error", + }, + }); + + try { + faunaToCommandError(serviceError); + } catch (error) { + expect(error).to.be.instanceOf(CommandError); + expect(error.cause).to.equal(serviceError); + } + }); + + it("should convert NetworkError to CommandError with network error message", () => { + const networkError = new NetworkError("Network failure"); + + try { + faunaToCommandError(networkError); + } catch (error) { + expect(error).to.be.instanceOf(CommandError); + expect(error.message).to.equal(NETWORK_ERROR_MESSAGE); + expect(error.cause).to.equal(networkError); + } + }); + + it("should pass through other errors unchanged", () => { + const genericError = new Error("Generic error"); + + try { + faunaToCommandError(genericError); + } catch (error) { + expect(error).to.equal(genericError); + } + }); + + it("should call custom handler if provided", () => { + let handlerCalled = false; + const serviceError = new ServiceError({ + error: { + code: "unauthorized", + message: "Invalid token", + }, + }); + + const handler = (e) => { + handlerCalled = true; + expect(e).to.equal(serviceError); + }; + + try { + faunaToCommandError(serviceError, handler); + } catch (error) { + expect(handlerCalled).to.be.true; + expect(error).to.be.instanceOf(AuthenticationError); + } + }); +}); diff --git a/test/lib/faunadb.mjs b/test/lib/faunadb.mjs new file mode 100644 index 00000000..22136d6d --- /dev/null +++ b/test/lib/faunadb.mjs @@ -0,0 +1,99 @@ +import { expect } from "chai"; +import faunadb from "faunadb"; + +import { + AuthenticationError, + AuthorizationError, + CommandError, + NETWORK_ERROR_MESSAGE, +} from "../../src/lib/errors.mjs"; +import { faunadbToCommandError } from "../../src/lib/faunadb.mjs"; + +describe("faunadbToCommandError", () => { + it("should convert Unauthorized error to AuthenticationError", () => { + const faunaError = new faunadb.errors.FaunaHTTPError("Unauthorized", { + responseContent: { + errors: [], + }, + }); + + expect(() => faunadbToCommandError(faunaError)).to.throw( + AuthenticationError, + ); + }); + + it("should convert PermissionDenied error to AuthorizationError", () => { + const faunaError = new faunadb.errors.FaunaHTTPError("PermissionDenied", { + responseContent: { + errors: [], + }, + }); + + expect(() => faunadbToCommandError(faunaError)).to.throw( + AuthorizationError, + ); + }); + + it("should convert BadRequest error to CommandError", () => { + const faunaError = new faunadb.errors.FaunaHTTPError("BadRequest", { + responseContent: { + errors: [], + }, + }); + + expect(() => faunadbToCommandError(faunaError)).to.throw(CommandError); + }); + + it("should convert NotFound error to CommandError", () => { + const faunaError = new faunadb.errors.FaunaHTTPError("NotFound", { + responseContent: { + errors: [], + }, + }); + + expect(() => faunadbToCommandError(faunaError)).to.throw(CommandError); + }); + + it("should convert network error to CommandError with network message", () => { + const networkError = new TypeError("fetch failed"); + + expect(() => faunadbToCommandError(networkError)).to.throw( + CommandError, + NETWORK_ERROR_MESSAGE, + ); + }); + + it("should pass through other FaunaHTTPErrors unchanged", () => { + const faunaError = new faunadb.errors.FaunaHTTPError("Internal error", { + responseContent: { + errors: [], + }, + }); + + expect(() => faunadbToCommandError(faunaError)).to.throw( + faunadb.errors.FaunaHTTPError, + ); + }); + + it("should pass through other errors unchanged", () => { + const genericError = new Error("Generic error"); + + expect(() => faunadbToCommandError(genericError)).to.throw(Error); + }); + + it("should call optional error handler if provided", () => { + let handlerCalled = false; + const handler = () => { + handlerCalled = true; + }; + const error = new Error("Test error"); + + try { + faunadbToCommandError(error, handler); + } catch (e) { + // Expected to throw + } + + expect(handlerCalled).to.be.true; + }); +}); diff --git a/test/schema/schema.mjs b/test/schema/schema.mjs index a108a1c5..ff8980b9 100644 --- a/test/schema/schema.mjs +++ b/test/schema/schema.mjs @@ -2,10 +2,14 @@ import { expect } from "chai"; import chalk from "chalk"; +import sinon from "sinon"; import { builtYargs, run } from "../../src/cli.mjs"; import { setupTestContainer as setupContainer } from "../../src/config/setup-test-container.mjs"; -import { NETWORK_ERROR_MESSAGE } from "../../src/lib/errors.mjs"; +import { + AUTHENTICATION_ERROR_MESSAGE, + NETWORK_ERROR_MESSAGE, +} from "../../src/lib/errors.mjs"; describe("schema", function () { let container, logger, stderr; @@ -35,11 +39,13 @@ describe("schema", function () { }); it("can handle network errors", async function () { - // Schema push requires fsl locally...we need to accommodate for that, but for now, we'll just skip it - if (command === "schema push") { - return; - } container.resolve("fetch").rejects(new TypeError("fetch failed")); + container.resolve("gatherFSL").resolves([ + { + name: "test.fsl", + content: "collection Test { name: String }", + }, + ]); try { await run(`${command} --secret=test-secret --dir=test-dir`, container); @@ -48,6 +54,30 @@ describe("schema", function () { await stderr.waitForWritten(); expect(stderr.getWritten()).to.contain(NETWORK_ERROR_MESSAGE); + expect(stderr.getWritten()).to.not.contain("unexpected error"); + }); + + it("can handle unauthorized errors", async function () { + container.resolve("fetch").resolves({ + status: 401, + json: () => Promise.resolve({ error: { code: "unauthorized" } }), + }); + + container.resolve("gatherFSL").resolves([ + { + name: "test.fsl", + content: "collection Test { name: String }", + }, + ]); + + try { + await run(`${command} --secret=test-secret`, container); + } catch (e) {} + + expect(logger.stderr).to.have.been.calledWith( + sinon.match(AUTHENTICATION_ERROR_MESSAGE), + ); + expect(stderr.getWritten()).to.not.contain("unexpected error"); }); }); }); diff --git a/test/shell.mjs b/test/shell.mjs index 0962d207..f30959c6 100644 --- a/test/shell.mjs +++ b/test/shell.mjs @@ -123,6 +123,7 @@ describe("shell", function () { await stderr.waitForWritten(); expect(stderr.getWritten()).to.contain(NETWORK_ERROR_MESSAGE); + expect(stderr.getWritten()).to.not.contain("failed unexpectedly"); }); describe("history", function () {