From fd52552593e8e41166009f8bf3993d793eda6574 Mon Sep 17 00:00:00 2001 From: "E. Cooper" Date: Fri, 13 Dec 2024 13:23:31 -0800 Subject: [PATCH] Clean up error handling to have less unexpected errors --- 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 f8845de1..58994361 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"; @@ -61,10 +59,7 @@ async function listDatabasesWithSecret(argv) { .resolve("logger") .stdout(formatQueryResponse(result, { format: Format.JSON, color })); } catch (e) { - if (e instanceof FaunaError) { - throwForError(e); - } - throw e; + 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 () {