Skip to content

Commit

Permalink
Handle network errors with better messaging (#519)
Browse files Browse the repository at this point in the history
Co-authored-by: James Rodewig <[email protected]>
  • Loading branch information
ecooper and jrodewig authored Dec 13, 2024
1 parent 8ac0681 commit c963ec9
Show file tree
Hide file tree
Showing 8 changed files with 118 additions and 37 deletions.
15 changes: 13 additions & 2 deletions src/lib/db.mjs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//@ts-check

import { container } from "../cli.mjs";
import { CommandError } from "./errors.mjs";
import { CommandError, NETWORK_ERROR_MESSAGE } from "./errors.mjs";
import { retryInvalidCredsOnce } from "./fauna-client.mjs";

function buildParamsString({ argv, params, path }) {
Expand Down Expand Up @@ -31,6 +31,7 @@ function buildParamsString({ argv, params, path }) {
/**
* @param {fetchParameters} args
*/
// eslint-disable-next-line complexity
export async function makeFaunaRequest({
argv,
path,
Expand Down Expand Up @@ -60,7 +61,17 @@ export async function makeFaunaRequest({

if (body) fetchArgs.body = body;

const response = await fetch(fullUrl, fetchArgs);
let response;
try {
response = await fetch(fullUrl, fetchArgs);
} catch (err) {
if (err.name === "TypeError" && err.message.includes("fetch failed")) {
throw new CommandError(NETWORK_ERROR_MESSAGE);
}

throw err;
}

const obj = await response.json();

if (obj.error && shouldThrow) {
Expand Down
5 changes: 5 additions & 0 deletions src/lib/errors.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ import { container } from "../cli.mjs";

const BUG_REPORT_MESSAGE = `If you believe this is a bug, please report this issue on GitHub: https://github.com/fauna/fauna-shell/issues`;

// This error message is used in a few places where we handle network errors.
export const NETWORK_ERROR_MESSAGE =
"Unable to connect to Fauna due to a network error. If using --local, " +
"ensure your container is running with this command: fauna local";

/*
* These are the error message prefixes that yargs throws during
* validation. To detect these errors, you can either parse the stack
Expand Down
45 changes: 30 additions & 15 deletions src/lib/fauna-client.mjs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//@ts-check

import { container } from "../cli.mjs";
import { ValidationError } from "./errors.mjs";
import { isUnknownError, ValidationError } from "./errors.mjs";
import { colorize, Format } from "./formatting/colorize.mjs";

const SUMMARY_FQL_REGEX = /^(\s\s\|)|(\d\s\|)/;
Expand Down Expand Up @@ -88,20 +88,6 @@ export const runQueryFromString = (expression, argv) => {
}
};

/**
* Check if a database can be queried based on the current arguments.
* If it can't, it will throw an error.
* @param {*} argv
*/
export const isQueryable = async (argv) => {
const runQueryFromString = container.resolve("runQueryFromString");
try {
await runQueryFromString("1+1", argv);
} catch (err) {
throw new ValidationError(err.message, { cause: err });
}
};

/**
* Formats an error.
* @param {object} err - The error to format
Expand All @@ -122,6 +108,35 @@ export const formatError = (err, { apiVersion, raw, color }) => {
}
};

/**
* Check if a database can be queried based on the current arguments.
* If it can't, it will throw an error.
* @param {*} argv
*/
export const isQueryable = async (argv) => {
const runQueryFromString = container.resolve("runQueryFromString");
try {
await runQueryFromString("1+1", argv);
} catch (err) {
if (!isUnknownError(err)) {
throw err;
}

throw new ValidationError(
formatError(err, {
apiVersion: argv.apiVersion,
raw: false,
color: false,
}),
{
cause: err,
},
);
}

return true;
};

/**
* Formats a query response.
* @param {object} res - The query response
Expand Down
6 changes: 5 additions & 1 deletion src/lib/fauna.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
} from "fauna";

import { container } from "../cli.mjs";
import { ValidationError } from "./errors.mjs";
import { NETWORK_ERROR_MESSAGE, ValidationError } from "./errors.mjs";
import { formatQuerySummary } from "./fauna-client.mjs";
import { colorize, Format } from "./formatting/colorize.mjs";

Expand Down Expand Up @@ -147,6 +147,10 @@ export const formatError = (err, opts = {}) => {
// Otherwise, return the summary and fall back to the message.
return `${chalk.red("The query failed with the following error:")}\n\n${formatQuerySummary(err.queryInfo?.summary) ?? err.message}`;
} else {
if (err.name === "NetworkError") {
return `The query failed unexpectedly with the following error:\n\n${NETWORK_ERROR_MESSAGE}`;
}

return `The query failed unexpectedly with the following error:\n\n${err.message}`;
}
};
Expand Down
16 changes: 13 additions & 3 deletions src/lib/faunadb.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import { createContext, runInContext } from "node:vm";

import { container } from "../cli.mjs";
import { NETWORK_ERROR_MESSAGE } from "./errors.mjs";
import { colorize, Format } from "./formatting/colorize.mjs";

/**
Expand Down Expand Up @@ -103,22 +104,31 @@ export const formatError = (err, opts = {}) => {
return colorize(err, { color, format: Format.JSON });
}

const errorPrefix = "The query failed with the following error:\n\n";
const { errors } = err.requestResult.responseContent;
if (!errors) {
return colorize(err.message, { color });
return colorize(errorPrefix + err.message, { color });
}

const messages = [];
errors.forEach(({ code, description, position }) => {
messages.push(`${code}: ${description} at ${position.join(", ")}\n`);
});

return colorize(messages.join("\n").trim(), {
return colorize(errorPrefix + messages.join("\n").trim(), {
color,
});
}

return colorize(err.message, { color });
const errorPrefix =
"The query failed unexpectedly with the following error:\n\n";

// When fetch fails, we get a TypeError with a "fetch failed" message.
if (err.name === "TypeError" && err.message.includes("fetch failed")) {
return colorize(errorPrefix + NETWORK_ERROR_MESSAGE, { color });
}

return colorize(errorPrefix + err.message, { color });
};

/**
Expand Down
28 changes: 14 additions & 14 deletions test/query.mjs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
//@ts-check

import { expect } from "chai";
import { ServiceError } from "fauna";
import { NetworkError, ServiceError } from "fauna";
import sinon from "sinon";

import { 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 { colorize } from "../src/lib/formatting/colorize.mjs";
import {
createV4QueryFailure,
Expand Down Expand Up @@ -171,19 +172,6 @@ describe("query", function () {
colorize([], { format: "json", color: false }),
);
});

// This test is disabled because the argv fallback requires a real process.argv
// and there's no way blessed way to override it in the test environment.
it.skip("can mute stderr if --quiet is used", async function () {
runQueryFromString.rejects(new Error("test error"));

try {
await run(`query "Database.all()" --quiet --secret=foo`, container);
} catch (e) {}

expect(logger.stdout).to.not.be.called;
expect(logger.stderr).to.not.be.called;
});
});

describe("--local usage", function () {
Expand Down Expand Up @@ -390,6 +378,18 @@ describe("query", function () {
expect(logger.stderr).to.not.be.called;
expect(logger.stdout).to.have.been.calledWith(sinon.match(/fql/));
});

it("can handle network errors", async function () {
runQueryFromString.rejects(new NetworkError("test error", { cause: {} }));

try {
await run(`query "Database.all()" --local`, container);
} catch (e) {}

expect(logger.stderr).to.have.been.calledWith(
sinon.match(NETWORK_ERROR_MESSAGE),
);
});
});

describe("v4", function () {
Expand Down
20 changes: 19 additions & 1 deletion test/schema/schema.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ import chalk from "chalk";

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";

describe("schema", function () {
let container, logger;
let container, logger, stderr;
beforeEach(() => {
container = setupContainer();
logger = container.resolve("logger");
stderr = container.resolve("stderrStream");
});

[
Expand All @@ -31,5 +33,21 @@ describe("schema", function () {
);
expect(container.resolve("parseYargs")).to.have.been.calledOnce;
});

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"));

try {
await run(`${command} --secret=test-secret --dir=test-dir`, container);
} catch (e) {}

await stderr.waitForWritten();

expect(stderr.getWritten()).to.contain(NETWORK_ERROR_MESSAGE);
});
});
});
20 changes: 19 additions & 1 deletion test/shell.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ import path from "node:path";

import * as awilix from "awilix";
import { expect } from "chai";
import { NetworkError } from "fauna";
import sinon, { stub } from "sinon";

import { run } from "../src/cli.mjs";
import { setupTestContainer as setupContainer } from "../src/config/setup-test-container.mjs";
import { ValidationError } from "../src/lib/errors.mjs";
import { NETWORK_ERROR_MESSAGE, ValidationError } from "../src/lib/errors.mjs";
import { isQueryable } from "../src/lib/fauna-client.mjs";
import { dirExists } from "../src/lib/file-util.mjs";
import { colorize } from "../src/lib/formatting/colorize.mjs";
import { createV4QuerySuccess, createV10QuerySuccess } from "./helpers.mjs";
Expand Down Expand Up @@ -107,6 +109,22 @@ describe("shell", function () {
expect(stderr.getWritten()).to.match(/Database not found: us\/bad/);
});

it("can handle network errors", async function () {
runQueryFromString.rejects(new NetworkError("test error", { cause: {} }));
container.register({
isQueryable: awilix.asValue(isQueryable),
});
const runPromise = run(`shell --format json -d us/bad`, container);

try {
await runPromise;
} catch {}

await stderr.waitForWritten();

expect(stderr.getWritten()).to.contain(NETWORK_ERROR_MESSAGE);
});

describe("history", function () {
const upArrow = "\x1b[A";
const downArrow = "\x1b[B";
Expand Down

0 comments on commit c963ec9

Please sign in to comment.