Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up error handling to have less unexpected errors #529

Merged
merged 3 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 24 additions & 21 deletions src/commands/database/create.mjs
Original file line number Diff line number Diff line change
@@ -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";

Expand Down Expand Up @@ -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 },
);
}
});
}
}

Expand Down
19 changes: 10 additions & 9 deletions src/commands/database/delete.mjs
Original file line number Diff line number Diff line change
@@ -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) {
Expand All @@ -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.`,
);
}
});
}
}

Expand Down
9 changes: 2 additions & 7 deletions src/commands/database/list.mjs
Original file line number Diff line number Diff line change
@@ -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";

Expand Down Expand Up @@ -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);
}
}

Expand Down
72 changes: 52 additions & 20 deletions src/lib/account.mjs
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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<Response | Object>} - 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;
}
5 changes: 2 additions & 3 deletions src/lib/auth/accountKeys.mjs
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -82,7 +81,7 @@
async getOrRefreshKey() {
if (this.keySource === "credentials-file") {
const key = this.keyStore.get();
// TODO: track ttl for account and refresh keys

Check warning on line 84 in src/lib/auth/accountKeys.mjs

View workflow job for this annotation

GitHub Actions / lint

Unexpected 'todo' comment: 'TODO: track ttl for account and refresh...'
if (!key) {
this.logger.debug(
"Found account key, but it is expired. Refreshing...",
Expand Down Expand Up @@ -118,7 +117,7 @@
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;
Expand Down
13 changes: 8 additions & 5 deletions src/lib/db.mjs
Original file line number Diff line number Diff line change
@@ -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 }) {
Expand Down Expand Up @@ -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;
}

Expand Down
69 changes: 68 additions & 1 deletion src/lib/errors.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 (
Expand Down
6 changes: 3 additions & 3 deletions src/lib/fauna-account-client.mjs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -20,7 +20,7 @@
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",
Expand All @@ -31,7 +31,7 @@
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",
Expand Down Expand Up @@ -132,7 +132,7 @@
* @throws {Error} - Throws an error if there is an issue during session retrieval.
*/

// TODO: get/set expiration details

Check warning on line 135 in src/lib/fauna-account-client.mjs

View workflow job for this annotation

GitHub Actions / lint

Unexpected 'todo' comment: 'TODO: get/set expiration details'
static async getSession(accessToken) {
const makeAccountRequest = container.resolve("makeAccountRequest");
try {
Expand All @@ -149,7 +149,7 @@
}
}

// TODO: get/set expiration details

Check warning on line 152 in src/lib/fauna-account-client.mjs

View workflow job for this annotation

GitHub Actions / lint

Unexpected 'todo' comment: 'TODO: get/set expiration details'
/**
* Uses refreshToken to get a new accountKey and refreshToken.
* @param {*} refreshToken
Expand Down
Loading
Loading