From c963ec9e6a2e2e7f302e2396f1cafb8d142733cb Mon Sep 17 00:00:00 2001 From: "E. Cooper" Date: Thu, 12 Dec 2024 16:42:55 -0800 Subject: [PATCH] Handle network errors with better messaging (#519) Co-authored-by: James Rodewig --- src/lib/db.mjs | 15 ++++++++++++-- src/lib/errors.mjs | 5 +++++ src/lib/fauna-client.mjs | 45 ++++++++++++++++++++++++++-------------- src/lib/fauna.mjs | 6 +++++- src/lib/faunadb.mjs | 16 +++++++++++--- test/query.mjs | 28 ++++++++++++------------- test/schema/schema.mjs | 20 +++++++++++++++++- test/shell.mjs | 20 +++++++++++++++++- 8 files changed, 118 insertions(+), 37 deletions(-) diff --git a/src/lib/db.mjs b/src/lib/db.mjs index b6891813..069abe57 100644 --- a/src/lib/db.mjs +++ b/src/lib/db.mjs @@ -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 }) { @@ -31,6 +31,7 @@ function buildParamsString({ argv, params, path }) { /** * @param {fetchParameters} args */ +// eslint-disable-next-line complexity export async function makeFaunaRequest({ argv, path, @@ -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) { diff --git a/src/lib/errors.mjs b/src/lib/errors.mjs index 339f9fc3..109fe0d6 100644 --- a/src/lib/errors.mjs +++ b/src/lib/errors.mjs @@ -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 diff --git a/src/lib/fauna-client.mjs b/src/lib/fauna-client.mjs index f13cc55c..eda7f957 100644 --- a/src/lib/fauna-client.mjs +++ b/src/lib/fauna-client.mjs @@ -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\|)/; @@ -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 @@ -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 diff --git a/src/lib/fauna.mjs b/src/lib/fauna.mjs index 7ec25ab4..dfc1b20d 100644 --- a/src/lib/fauna.mjs +++ b/src/lib/fauna.mjs @@ -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"; @@ -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}`; } }; diff --git a/src/lib/faunadb.mjs b/src/lib/faunadb.mjs index 6043d3fe..fa027fce 100644 --- a/src/lib/faunadb.mjs +++ b/src/lib/faunadb.mjs @@ -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"; /** @@ -103,9 +104,10 @@ 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 = []; @@ -113,12 +115,20 @@ export const formatError = (err, opts = {}) => { 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 }); }; /** diff --git a/test/query.mjs b/test/query.mjs index e306c2b6..c33580fb 100644 --- a/test/query.mjs +++ b/test/query.mjs @@ -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, @@ -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 () { @@ -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 () { diff --git a/test/schema/schema.mjs b/test/schema/schema.mjs index 92dc8d48..a108a1c5 100644 --- a/test/schema/schema.mjs +++ b/test/schema/schema.mjs @@ -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"); }); [ @@ -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); + }); }); }); diff --git a/test/shell.mjs b/test/shell.mjs index 3669316e..0962d207 100644 --- a/test/shell.mjs +++ b/test/shell.mjs @@ -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"; @@ -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";