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

Handle network errors with better messaging #519

Merged
merged 5 commits into from
Dec 13, 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
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
Loading