From 2b9c521a6134e3b33011db640e090f8c7e399d74 Mon Sep 17 00:00:00 2001 From: Cleve Stuart <90649124+cleve-fauna@users.noreply.github.com> Date: Fri, 13 Dec 2024 06:36:43 -0800 Subject: [PATCH 1/2] Detect if port for local Fauna is already occupied. Allow users to control hostIp. (#512) * Tests for health checks. * Argument checks * Skip pull test * Tests for pull failing * Tests for pull failing * Support for hostIp * Port occupied tests * Remove only * Fail gracefully if the container is already present but on a different port. * Doc fixes --- src/commands/local.mjs | 82 +++++--- src/config/setup-container.mjs | 2 + src/config/setup-test-container.mjs | 3 + src/lib/docker-containers.mjs | 160 +++++++++++---- src/lib/errors.mjs | 7 +- test/local.mjs | 304 +++++++++++++++++++++++++++- 6 files changed, 481 insertions(+), 77 deletions(-) diff --git a/src/commands/local.mjs b/src/commands/local.mjs index 5a3f7c84..fb6c763b 100644 --- a/src/commands/local.mjs +++ b/src/commands/local.mjs @@ -1,4 +1,5 @@ import { ensureContainerRunning } from "../lib/docker-containers.mjs"; +import { CommandError } from "../lib/errors.mjs"; /** * Starts the local Fauna container @@ -10,9 +11,12 @@ async function startLocal(argv) { await ensureContainerRunning({ imageName: argv.image, containerName: argv.name, + hostIp: argv.hostIp, hostPort: argv.hostPort, containerPort: argv.containerPort, pull: argv.pull, + interval: argv.interval, + maxAttempts: argv.maxAttempts, }); } @@ -22,29 +26,61 @@ async function startLocal(argv) { * @returns {import('yargs').Argv} The yargs instance */ function buildLocalCommand(yargs) { - return yargs.options({ - containerPort: { - describe: "The port inside the container Fauna listens on.", - type: "number", - default: "8443", - }, - hostPort: { - describe: - "The port on the host machine mapped to the container's port. This is the port you'll connect to Fauna on.", - type: "number", - default: "8443", - }, - name: { - describe: "The name to give the container", - type: "string", - default: "faunadb", - }, - pull: { - describe: "Pull the latest image before starting the container.", - type: "boolean", - default: true, - }, - }); + return yargs + .options({ + containerPort: { + describe: "The port inside the container Fauna listens on.", + type: "number", + default: 8443, + }, + hostPort: { + describe: + "The port on the host machine mapped to the container's port. This is the port you'll connect to Fauna on.", + type: "number", + default: 8443, + }, + hostIp: { + describe: `The IP address to bind the container's exposed port on the host.`, + type: "string", + default: "0.0.0.0", + }, + interval: { + describe: + "The interval (in milliseconds) between health check attempts. Determines how often the CLI checks if the Fauna container is ready.", + type: "number", + default: 10000, + }, + maxAttempts: { + describe: + "The maximum number of health check attempts before declaring the start Fauna continer process as failed.", + type: "number", + default: 100, + }, + name: { + describe: "The name to give the container", + type: "string", + default: "faunadb", + }, + pull: { + describe: "Pull the latest image before starting the container.", + type: "boolean", + default: true, + }, + }) + .check((argv) => { + if (argv.maxAttempts < 1) { + throw new CommandError("--maxAttempts must be greater than 0.", { + hideHelp: false, + }); + } + if (argv.interval < 0) { + throw new CommandError( + "--interval must be greater than or equal to 0.", + { hideHelp: false }, + ); + } + return true; + }); } export default { diff --git a/src/config/setup-container.mjs b/src/config/setup-container.mjs index 2ec113e8..e9d14a60 100644 --- a/src/config/setup-container.mjs +++ b/src/config/setup-container.mjs @@ -1,5 +1,6 @@ import fs from "node:fs"; import * as fsp from "node:fs/promises"; +import net from "node:net"; import os from "node:os"; import path from "node:path"; import { exit } from "node:process"; @@ -60,6 +61,7 @@ export const injectables = { fetch: awilix.asValue(fetchWrapper), fs: awilix.asValue(fs), fsp: awilix.asValue(fsp), + net: awilix.asValue(net), dirname: awilix.asValue(path.dirname), normalize: awilix.asValue(path.normalize), homedir: awilix.asValue(os.homedir), diff --git a/src/config/setup-test-container.mjs b/src/config/setup-test-container.mjs index 95f26151..426863bf 100644 --- a/src/config/setup-test-container.mjs +++ b/src/config/setup-test-container.mjs @@ -1,4 +1,5 @@ import fs from "node:fs"; +import net from "node:net"; import path from "node:path"; import { PassThrough } from "node:stream"; @@ -43,6 +44,7 @@ export function setupTestContainer() { const thingsToManuallyMock = automock(container); const customfs = stub({ ...fs }); + const customNet = stub({ ...net }); // this is a mock used by the default profile behavior customfs.readdirSync.withArgs(process.cwd()).returns([]); @@ -58,6 +60,7 @@ export function setupTestContainer() { // real implementation parseYargs: awilix.asValue(spy(parseYargs)), fs: awilix.asValue(customfs), + net: awilix.asValue(customNet), homedir: awilix.asValue( stub().returns(path.join(__dirname, "../../test/test-homedir")), ), diff --git a/src/lib/docker-containers.mjs b/src/lib/docker-containers.mjs index 41f3485a..17bb7d0a 100644 --- a/src/lib/docker-containers.mjs +++ b/src/lib/docker-containers.mjs @@ -1,22 +1,28 @@ import { container } from "../cli.mjs"; -import { CommandError } from "./errors.mjs"; +import { CommandError, SUPPORT_MESSAGE } from "./errors.mjs"; const IMAGE_NAME = "fauna/faunadb:latest"; /** * Ensures the container is running - * @param {string} imageName The name of the image to create the container from - * @param {string} containerName The name of the container to start - * @param {number} hostPort The port on the host machine mapped to the container's port - * @param {number} containerPort The port inside the container Fauna listens on - * @param {boolean} pull Whether to pull the latest image + * @param {Object} options The options object + * @param {string} options.containerName The name of the container to start + * @param {string} options.hostIp The IP address to bind the container's exposed port on the host + * @param {number} options.hostPort The port on the host machine mapped to the container's port + * @param {number} options.containerPort The port inside the container Fauna listens on + * @param {boolean} options.pull Whether to pull the latest image + * @param {number} [options.interval] The interval (in milliseconds) between health check attempts + * @param {number} [options.maxAttempts] The maximum number of health check attempts before declaring the start Fauna continer process as failed * @returns {Promise} */ export async function ensureContainerRunning({ containerName, + hostIp, hostPort, containerPort, pull, + interval, + maxAttempts, }) { const logger = container.resolve("logger"); if (pull) { @@ -25,6 +31,7 @@ export async function ensureContainerRunning({ const logStream = await startContainer({ imageName: IMAGE_NAME, containerName, + hostIp, hostPort, containerPort, }); @@ -32,8 +39,10 @@ export async function ensureContainerRunning({ `[StartContainer] Container '${containerName}' started. Monitoring HealthCheck for readiness.`, ); await waitForHealthCheck({ - url: `http://localhost:${hostPort}`, + url: `http://${hostIp}:${hostPort}`, logStream, + interval, + maxAttempts, }); logger.stderr( `[ContainerReady] Container '${containerName}' is up and healthy.`, @@ -84,10 +93,10 @@ async function pullImage(imageName) { ); }); } catch (error) { - logger.stderr( - `[PullImage] Error pulling image ${imageName}: ${error.message}`, + throw new CommandError( + `[PullImage] Failed to pull image '${imageName}': ${error.message}. ${SUPPORT_MESSAGE}`, + { cause: error }, ); - throw error; } } @@ -117,45 +126,105 @@ function writePullProgress(layers, numLines) { /** * Finds a container by name - * @param {string} containerName The name of the container to find + * @param {Object} options The options object + * @param {string} options.containerName The name of the container to find + * @param {number} options.hostPort The port to check * @returns {Promise} The container object if found, otherwise undefined. * The container object has the following properties: * - Id: The ID of the container * - Names: The names of the container * - State: The state of the container */ -async function findContainer(containerName) { +async function findContainer({ containerName, hostPort }) { const docker = container.resolve("docker"); const logger = container.resolve("logger"); // Dependency injection for logger - logger.stderr( - `[GetContainerState] Checking state for container '${containerName}'...`, - ); + logger.stderr(`[FindContainer] Looking for container '${containerName}'...`); const filters = JSON.stringify({ name: [containerName] }); const containers = await docker.listContainers({ all: true, filters }); - return containers.length > 0 ? containers[0] : null; + if (containers.length === 0) { + return null; + } + const result = containers[0]; + const diffPort = result.Ports.find( + (c) => c.PublicPort !== undefined && c.PublicPort !== hostPort, + ); + if (diffPort) { + throw new CommandError( + `[FindContainer] Container '${containerName}' is already \ +in use on hostPort '${diffPort.PublicPort}'. Please use a new name via \ +arguments --name --hostPort ${hostPort} to start the container.`, + { hideHelp: false }, + ); + } + return result; +} + +/** + * Checks if a port is occupied. + * @param {Object} options The options object + * @param {number} options.hostPort The port to check + * @param {string} options.hostIp The IP address to bind the container's exposed port on the host. + * @returns {Promise} a promise that resolves to true if the port is occupied, false otherwise. + */ +async function isPortOccupied({ hostPort, hostIp }) { + const net = container.resolve("net"); + return new Promise((resolve, reject) => { + const server = net.createServer(); + server.once("error", (err) => { + if (err.code === "EADDRINUSE") { + resolve(true); // Port is occupied + } else { + reject(err); // Some other error occurred + } + }); + + server.on("listening", () => { + server.close(() => { + resolve(false); // Port is free + }); + }); + + server.listen(hostPort, hostIp); + }); } /** * Creates a container - * @param {string} imageName The name of the image to create the container from - * @param {string} containerName The name of the container to start - * @param {number} hostPort The port on the host machine mapped to the container's port - * @param {number} containerPort The port inside the container Fauna listens on + * @param {Object} options The options object + * @param {string} options.imageName The name of the image to create the container from + * @param {string} options.containerName The name of the container to start + * @param {string} options.hostIp The IP address to bind the container's exposed port on the host + * @param {number} options.hostPort The port on the host machine mapped to the container's port + * @param {number} options.containerPort The port inside the container Fauna listens on * @returns {Promise} The container object */ async function createContainer({ imageName, containerName, + hostIp, hostPort, containerPort, }) { const docker = container.resolve("docker"); + const occupied = await isPortOccupied({ hostIp, hostPort }); + if (occupied) { + throw new CommandError( + `[StartContainer] The hostPort '${hostPort}' on IP '${hostIp}' is already occupied. \ +Please pass a --hostPort other than '${hostPort}'.`, + { hideHelp: false }, + ); + } const dockerContainer = await docker.createContainer({ Image: imageName, name: containerName, HostConfig: { PortBindings: { - [`${containerPort}/tcp`]: [{ HostPort: hostPort }], + [`${containerPort}/tcp`]: [ + { + HostPort: `${hostPort}`, + HostIp: hostIp, + }, + ], }, AutoRemove: true, }, @@ -168,21 +237,24 @@ async function createContainer({ /** * Starts a container and returns a log stream if the container is not yet running. - * @param {string} imageName The name of the image to create the container from - * @param {string} containerName The name of the container to start - * @param {number} hostPort The port on the host machine mapped to the container's port - * @param {number} containerPort The port inside the container Fauna listens on + * @param {Object} options The options object + * @param {string} options.imageName The name of the image to create the container from + * @param {string} options.containerName The name of the container to start + * @param {string} options.hostIp The IP address to bind the container's exposed port on the host. + * @param {number} options.hostPort The port on the host machine mapped to the container's port + * @param {number} options.containerPort The port inside the container Fauna listens on * @returns {Promise} The log stream */ async function startContainer({ imageName, containerName, + hostIp, hostPort, containerPort, }) { const docker = container.resolve("docker"); const logger = container.resolve("logger"); - const existingContainer = await findContainer(containerName); + const existingContainer = await findContainer({ containerName, hostPort }); let logStream = undefined; if (existingContainer) { const dockerContainer = docker.getContainer(existingContainer.Id); @@ -219,6 +291,7 @@ async function startContainer({ const dockerContainer = await createContainer({ imageName, containerName, + hostIp, hostPort, containerPort, }); @@ -233,8 +306,9 @@ async function startContainer({ /** * Creates a log stream for the container - * @param {Object} dockerContainer The container object - * @param {string} containerName The name of the container + * @param {Object} options The options object + * @param {Object} options.dockerContainer The container object + * @param {string} options.containerName The name of the container * @returns {Promise} The log stream */ async function createLogStream({ dockerContainer, containerName }) { @@ -272,17 +346,18 @@ async function createLogStream({ dockerContainer, containerName }) { /** * Waits for the container to be ready - * @param {string} url The url to check - * @param {number} maxAttempts The maximum number of attempts to check - * @param {number} delay The delay between attempts in milliseconds - * @param {Object} logStream The log stream to destroy when the container is ready + * @param {Object} options The options object + * @param {string} options.url The url to check + * @param {number} [options.maxAttempts=100] The maximum number of attempts to check + * @param {number} [options.interval=10000] The interval between attempts in milliseconds + * @param {Object} options.logStream The log stream to destroy when the container is ready * @returns {Promise} a promise that resolves when the container is ready. * It will reject if the container is not ready after the maximum number of attempts. */ async function waitForHealthCheck({ url, maxAttempts = 100, - delay = 10000, + interval = 10000, logStream, }) { const logger = container.resolve("logger"); @@ -290,7 +365,7 @@ async function waitForHealthCheck({ logger.stderr(`[HealthCheck] Waiting for Fauna to be ready at ${url}...`); let attemptCounter = 0; - + let errorMessage = ""; while (attemptCounter < maxAttempts) { try { /* eslint-disable-next-line no-await-in-loop */ @@ -303,23 +378,24 @@ async function waitForHealthCheck({ logStream?.destroy(); return; } - } catch (error) { - logger.stderr( - `[HealthCheck] Fauna is not yet ready. Attempt ${attemptCounter + 1}/${maxAttempts} failed: ${error.message}. Retrying in ${delay / 1000} seconds...`, - ); + errorMessage = `with HTTP status: '${response.status}'`; + } catch (e) { + errorMessage = `with error: ${e.message}`; } - + logger.stderr( + `[HealthCheck] Fauna is not yet ready. Attempt ${attemptCounter + 1}/${maxAttempts} failed ${errorMessage}. Retrying in ${interval / 1000} seconds...`, + ); attemptCounter++; /* eslint-disable-next-line no-await-in-loop */ await new Promise((resolve) => { - setTimeout(resolve, delay); + setTimeout(resolve, interval); }); } logger.stderr( `[HealthCheck] Max attempts reached. Service at ${url} did not respond.`, ); - throw new Error( - `[HealthCheck] Fauna at ${url} is not ready after ${maxAttempts} attempts.`, + throw new CommandError( + `[HealthCheck] Fauna at ${url} is not ready after ${maxAttempts} attempts. Consider increasing --interval or --maxAttempts.`, ); } diff --git a/src/lib/errors.mjs b/src/lib/errors.mjs index 109fe0d6..877a758e 100644 --- a/src/lib/errors.mjs +++ b/src/lib/errors.mjs @@ -4,7 +4,10 @@ import util from "util"; 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`; +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"; // This error message is used in a few places where we handle network errors. export const NETWORK_ERROR_MESSAGE = @@ -117,6 +120,8 @@ export const handleParseYargsError = async ( logger.debug(`unknown error thrown: ${e.name}`, "error"); logger.debug(util.inspect(e, true, 2, false), "error"); } else { + logger.debug(`known error thrown: ${e.name}`, "error"); + logger.debug(util.inspect(e, true, 2, false), "error"); // Otherwise, just use the error message subMessage = hasAnsi(e.message) ? e.message : chalk.red(e.message); } diff --git a/test/local.mjs b/test/local.mjs index 9046ef4c..72f22a04 100644 --- a/test/local.mjs +++ b/test/local.mjs @@ -14,10 +14,13 @@ describe("ensureContainerRunning", () => { stderrStream, docker, logsStub, + serverMock, + simulateError, startStub, unpauseStub; beforeEach(async () => { + simulateError = false; container = await setupTestContainer(); logger = container.resolve("logger"); stderrStream = container.resolve("stderrStream"); @@ -26,9 +29,80 @@ describe("ensureContainerRunning", () => { logsStub = stub(); startStub = stub(); unpauseStub = stub(); + // Requested port is free + serverMock = { + close: sinon.stub(), + once: sinon.stub(), + on: sinon.stub(), + listen: sinon.stub(), + }; + serverMock.listen.callsFake(() => { + if (simulateError) { + // Trigger the error callback + const errorCallback = serverMock.once.withArgs("error").args[0]?.[1]; + if (errorCallback) { + /** @type {Error & {code?: string}} */ + const error = new Error("Foo"); + error.code = "EADDRINUSE"; + errorCallback(error); + } + } else { + // Trigger the listening callback + const listeningCallback = + serverMock.on.withArgs("listening").args[0]?.[1]; + if (listeningCallback) { + listeningCallback(); + } + } + }); + + serverMock.on.withArgs("listening").callsFake((event, callback) => { + if (simulateError) { + // Trigger the error callback + const errorCallback = serverMock.once.withArgs("error").args[0]?.[1]; + if (errorCallback) { + /** @type {Error & {code?: string}} */ + const error = new Error("Foo"); + error.code = "EADDRINUSE"; + errorCallback(error); + } + } else { + callback(); + } + }); + + serverMock.close.callsFake((callback) => { + if (callback) callback(); + }); + const net = container.resolve("net"); + net.createServer.returns(serverMock); }); - it.skip("handles argv tweaks correctly", () => {}); + it("Shows a clear error to the user if something is already running on the desired port.", async () => { + simulateError = true; + docker.pull.onCall(0).resolves(); + docker.modem.followProgress.callsFake((stream, onFinished) => { + onFinished(); + }); + docker.listContainers.onCall(0).resolves([]); + try { + // Run the actual command + await run("local", container); + throw new Error("Expected an error to be thrown."); + } catch (_) { + // Expected error, no action needed + } + + const written = stderrStream.getWritten(); + + // Assertions + expect(written).to.contain( + "[StartContainer] The hostPort '8443' on IP '0.0.0.0' is already occupied. \ +Please pass a --hostPort other than '8443'.", + ); + expect(written).to.contain("fauna local"); + expect(written).not.to.contain("An unexpected"); + }); it("Creates and starts a container when none exists", async () => { docker.pull.onCall(0).resolves(); @@ -60,7 +134,12 @@ describe("ensureContainerRunning", () => { name: "faunadb", HostConfig: { PortBindings: { - "8443/tcp": [{ HostPort: "8443" }], + "8443/tcp": [ + { + HostPort: "8443", + HostIp: "0.0.0.0", + }, + ], }, AutoRemove: true, }, @@ -73,14 +152,150 @@ describe("ensureContainerRunning", () => { ); }); + it("The user can control the hostIp, hostPort, containerPort, and name", async () => { + docker.pull.onCall(0).resolves(); + docker.modem.followProgress.callsFake((stream, onFinished) => { + onFinished(); + }); + docker.listContainers.onCall(0).resolves([]); + fetch.onCall(0).resolves(f({})); // fast succeed the health check + logsStub.callsFake(async () => ({ + on: () => {}, + destroy: () => {}, + })); + docker.createContainer.resolves({ + start: startStub, + logs: logsStub, + unpause: unpauseStub, + }); + await run( + "local --hostPort 10 --containerPort 11 --name Taco --hostIp 127.0.0.1", + container, + ); + expect(docker.createContainer).to.have.been.calledWith({ + Image: "fauna/faunadb:latest", + name: "Taco", + HostConfig: { + PortBindings: { + "11/tcp": [ + { + HostPort: "10", + HostIp: "127.0.0.1", + }, + ], + }, + AutoRemove: true, + }, + ExposedPorts: { + "11/tcp": {}, + }, + }); + }); + + it("Skips pull if --pull is false.", async () => { + docker.listContainers.onCall(0).resolves([]); + fetch.onCall(0).resolves(f({})); // fast succeed the health check + logsStub.callsFake(async () => ({ + on: () => {}, + destroy: () => {}, + })); + docker.createContainer.resolves({ + start: startStub, + logs: logsStub, + unpause: unpauseStub, + }); + await run("local --pull false", container); + expect(docker.pull).not.to.have.been.called; + expect(docker.modem.followProgress).not.to.have.been.called; + expect(startStub).to.have.been.called; + expect(logsStub).to.have.been.called; + expect(docker.createContainer).to.have.been.called; + expect(logger.stderr).to.have.been.calledWith( + "[ContainerReady] Container 'faunadb' is up and healthy.", + ); + }); + + it("Fails start with a prompt to contact Fauna if pull fails.", async () => { + docker.pull.onCall(0).rejects(new Error("Remote repository not found")); + docker.listContainers.onCall(0).resolves([]); + fetch.onCall(0).resolves(f({})); // fast succeed the health check + logsStub.callsFake(async () => ({ + on: () => {}, + destroy: () => {}, + })); + docker.createContainer.resolves({ + start: startStub, + logs: logsStub, + unpause: unpauseStub, + }); + try { + await run("local", container); + throw new Error("Expected an error to be thrown."); + } catch (_) {} + expect(docker.pull).to.have.been.called; + expect(docker.modem.followProgress).not.to.have.been.called; + expect(startStub).not.to.have.been.called; + expect(logsStub).not.to.have.been.called; + expect(docker.createContainer).not.to.have.been.called; + const written = stderrStream.getWritten(); + expect(written).to.contain( + `[PullImage] Failed to pull image 'fauna/faunadb:latest': Remote repository \ +not found. If this issue persists contact support: \ +https://support.fauna.com/hc/en-us/requests/new`, + ); + expect(written).not.to.contain("An unexpected"); + expect(written).not.to.contain("fauna local"); // help text + }); + + it("Throws an error if the health check fails", async () => { + docker.pull.onCall(0).resolves(); + docker.modem.followProgress.callsFake((stream, onFinished) => { + onFinished(); + }); + docker.listContainers.onCall(0).resolves([ + { + State: "created", + Names: ["/faunadb"], + Ports: [{ PublicPort: 8443 }], + }, + ]); + logsStub.callsFake(async () => ({ + on: () => {}, + destroy: () => {}, + })); + docker.getContainer.onCall(0).returns({ + logs: logsStub, + start: startStub, + unpause: unpauseStub, + }); + fetch.onCall(0).rejects(); + fetch.resolves(f({}, 503)); // fail from http + try { + await run("local --interval 0 --maxAttempts 3", container); + throw new Error("Expected an error to be thrown."); + } catch (_) {} + const written = stderrStream.getWritten(); + expect(written).to.contain("with HTTP status: '503'"); + expect(written).to.contain("with error:"); + expect(written).to.contain( + "[HealthCheck] Fauna at http://0.0.0.0:8443 is not ready after 3 attempts. Consider increasing --interval or --maxAttempts.", + ); + expect(written).not.to.contain("An unexpected"); + expect(written).not.to.contain("fauna local"); // help text + }); + it("exits if a container cannot be started", async () => { docker.pull.onCall(0).resolves(); docker.modem.followProgress.callsFake((stream, onFinished) => { onFinished(); }); - docker.listContainers - .onCall(0) - .resolves([{ State: "dead", Names: ["/faunadb"] }]); + docker.listContainers.onCall(0).resolves([ + { + State: "dead", + Names: ["/faunadb"], + Ports: [{ PublicPort: 8443 }], + }, + ]); fetch.onCall(0).resolves(f({})); // fast succeed the health check logsStub.callsFake(async () => ({ on: () => {}, @@ -102,6 +317,30 @@ describe("ensureContainerRunning", () => { expect(written).not.to.contain("An unexpected"); }); + it("throws an error if interval is less than 0", async () => { + try { + await run("local --interval -1", container); + throw new Error("Expected an error to be thrown."); + } catch (_) {} + const written = stderrStream.getWritten(); + expect(written).to.contain( + "--interval must be greater than or equal to 0.", + ); + expect(written).to.contain("fauna local"); // help text + expect(written).not.to.contain("An unexpected"); + }); + + it("throws an error if maxAttempts is less than 1", async () => { + try { + await run("local --maxAttempts 0", container); + throw new Error("Expected an error to be thrown."); + } catch (_) {} + const written = stderrStream.getWritten(); + expect(written).to.contain("--maxAttempts must be greater than 0."); + expect(written).to.contain("fauna local"); // help text + expect(written).not.to.contain("An unexpected"); + }); + [ { state: "paused", @@ -160,9 +399,13 @@ describe("ensureContainerRunning", () => { docker.modem.followProgress.callsFake((stream, onFinished) => { onFinished(); }); - docker.listContainers - .onCall(0) - .resolves([{ State: test.state, Names: ["/faunadb"] }]); + docker.listContainers.onCall(0).resolves([ + { + State: test.state, + Names: ["/faunadb"], + Ports: [{ PublicPort: 8443, Type: "tcp" }], + }, + ]); fetch.onCall(0).resolves(f({})); // fast succeed the health check logsStub.callsFake(async () => ({ on: () => {}, @@ -180,7 +423,7 @@ describe("ensureContainerRunning", () => { } expect(docker.pull).to.have.been.calledWith("fauna/faunadb:latest"); expect(docker.modem.followProgress).to.have.been.calledWith( - sinon.matchAny, + sinon.match.any, sinon.match.func, ); expect(docker.listContainers).to.have.been.calledWith({ @@ -199,14 +442,53 @@ describe("ensureContainerRunning", () => { "[StartContainer] Container 'faunadb' started. Monitoring HealthCheck for readiness.", ); expect(logger.stderr).to.have.been.calledWith( - "[HealthCheck] Waiting for Fauna to be ready at http://localhost:8443...", + "[HealthCheck] Waiting for Fauna to be ready at http://0.0.0.0:8443...", ); expect(logger.stderr).to.have.been.calledWith( - "[HealthCheck] Fauna is ready at http://localhost:8443", + "[HealthCheck] Fauna is ready at http://0.0.0.0:8443", ); expect(logger.stderr).to.have.been.calledWith( "[ContainerReady] Container 'faunadb' is up and healthy.", ); }); }); + + it("should throw if container exists with same name but different port", async () => { + const desiredPort = 8443; + docker.pull.onCall(0).resolves(); + docker.modem.followProgress.callsFake((stream, onFinished) => { + onFinished(); + }); + // Mock existing container with different port + docker.listContainers.onCall(0).resolves([ + { + Id: "mock-container-id", + Names: ["/faunadb"], + State: "running", + Ports: [ + { PublicPort: 9999, Type: "tcp" }, // Different port than desired + ], + }, + ]); + + try { + await run(`local --hostPort ${desiredPort}`, container); + throw new Error("Expected an error to be thrown."); + } catch (_) {} + expect(docker.listContainers).to.have.been.calledWith({ + all: true, + filters: JSON.stringify({ name: ["faunadb"] }), + }); + expect(startStub).not.to.have.been.called; + expect(unpauseStub).not.to.have.been.called; + expect(logsStub).not.to.have.been.called; + const written = stderrStream.getWritten(); + expect(written).to.contain( + `[FindContainer] Container 'faunadb' is already in use on hostPort '9999'. \ +Please use a new name via arguments --name --hostPort ${desiredPort} \ +to start the container.`, + ); + expect(written).not.to.contain("An unexpected"); + expect(written).to.contain("fauna local"); // help text + }); }); From 2f328039065b9d2fa7b3699ff819f1bf87f66803 Mon Sep 17 00:00:00 2001 From: Cleve Stuart <90649124+cleve-fauna@users.noreply.github.com> Date: Fri, 13 Dec 2024 08:48:28 -0800 Subject: [PATCH 2/2] Color log lines in fauna local command. (#524) * Color log lines in fauna local command. * Remove only * Test debug * Test debug * Test fixes? * Remove formatting tests --- src/commands/local.mjs | 1 + src/lib/docker-containers.mjs | 67 +++++++++++++++++-------------- src/lib/formatting/codeToAnsi.mjs | 5 ++- src/lib/formatting/colorize.mjs | 12 ++++++ test/local.mjs | 22 +++++----- 5 files changed, 63 insertions(+), 44 deletions(-) diff --git a/src/commands/local.mjs b/src/commands/local.mjs index fb6c763b..ec1b7ef3 100644 --- a/src/commands/local.mjs +++ b/src/commands/local.mjs @@ -17,6 +17,7 @@ async function startLocal(argv) { pull: argv.pull, interval: argv.interval, maxAttempts: argv.maxAttempts, + color: argv.color, }); } diff --git a/src/lib/docker-containers.mjs b/src/lib/docker-containers.mjs index 17bb7d0a..a9f2ff98 100644 --- a/src/lib/docker-containers.mjs +++ b/src/lib/docker-containers.mjs @@ -1,7 +1,9 @@ import { container } from "../cli.mjs"; import { CommandError, SUPPORT_MESSAGE } from "./errors.mjs"; +import { colorize, Format } from "./formatting/colorize.mjs"; const IMAGE_NAME = "fauna/faunadb:latest"; +let color = false; /** * Ensures the container is running @@ -23,8 +25,9 @@ export async function ensureContainerRunning({ pull, interval, maxAttempts, + color: _color, }) { - const logger = container.resolve("logger"); + color = _color; if (pull) { await pullImage(IMAGE_NAME); } @@ -35,7 +38,7 @@ export async function ensureContainerRunning({ hostPort, containerPort, }); - logger.stderr( + stderr( `[StartContainer] Container '${containerName}' started. Monitoring HealthCheck for readiness.`, ); await waitForHealthCheck({ @@ -44,9 +47,7 @@ export async function ensureContainerRunning({ interval, maxAttempts, }); - logger.stderr( - `[ContainerReady] Container '${containerName}' is up and healthy.`, - ); + stderr(`[ContainerReady] Container '${containerName}' is up and healthy.`); } /** @@ -57,8 +58,7 @@ export async function ensureContainerRunning({ */ async function pullImage(imageName) { const docker = container.resolve("docker"); - const logger = container.resolve("logger"); // Dependency injection for logger - logger.stderr(`[PullImage] Pulling image '${imageName}'...\n`); + stderr(`[PullImage] Pulling image '${imageName}'...`); try { const stream = await docker.pull(imageName); @@ -70,12 +70,12 @@ async function pullImage(imageName) { docker.modem.followProgress( stream, (err, output) => { - writePullProgress(layers, numLines); + writePullProgress(layers, numLines, imageName); if (err) { reject(err); } else { // Move to the reserved space for completion message - logger.stderr(`[PullImage] Image '${imageName}' pulled.`); + stderr(`[PullImage] Image '${imageName}' pulled.`); resolve(output); } }, @@ -86,7 +86,7 @@ async function pullImage(imageName) { `${event.id}: ${event.status} ${event.progress || ""}`; } if (Date.now() - lastUpdate > 100) { - numLines = writePullProgress(layers, numLines); + numLines = writePullProgress(layers, numLines, imageName); lastUpdate = Date.now(); } }, @@ -106,19 +106,21 @@ async function pullImage(imageName) { * so that the progress is displayed in the same place with no "flicker". * @param {Object} layers The layers of the image * @param {number} numLines The number of lines to clear and update + * @param {string} imageName The image name * @returns {number} The number of lines written. Pass this value back into * the next call to writePullProgress so that it can update the lines in place. */ -function writePullProgress(layers, numLines) { - const logger = container.resolve("logger"); +function writePullProgress(layers, numLines, imageName) { const stderrStream = container.resolve("stderrStream"); // Clear only the necessary lines and update them in place stderrStream.write(`\x1B[${numLines}A`); numLines = 0; // clear the screen stderrStream.write("\x1B[0J"); + stderr(`[PullImage] Pulling image '${imageName}'...`); + numLines++; Object.values(layers).forEach((line) => { - logger.stderr(line); + stderr(line); numLines++; }); return numLines; @@ -137,8 +139,7 @@ function writePullProgress(layers, numLines) { */ async function findContainer({ containerName, hostPort }) { const docker = container.resolve("docker"); - const logger = container.resolve("logger"); // Dependency injection for logger - logger.stderr(`[FindContainer] Looking for container '${containerName}'...`); + stderr(`[FindContainer] Looking for container '${containerName}'...`); const filters = JSON.stringify({ name: [containerName] }); const containers = await docker.listContainers({ all: true, filters }); if (containers.length === 0) { @@ -253,14 +254,13 @@ async function startContainer({ containerPort, }) { const docker = container.resolve("docker"); - const logger = container.resolve("logger"); const existingContainer = await findContainer({ containerName, hostPort }); let logStream = undefined; if (existingContainer) { const dockerContainer = docker.getContainer(existingContainer.Id); const state = existingContainer.State; if (state === "paused") { - logger.stderr( + stderr( `[StartContainer] Container '${containerName}' exists but is paused. Unpausing it...`, ); await dockerContainer.unpause(); @@ -269,7 +269,7 @@ async function startContainer({ containerName, }); } else if (state === "created" || state === "exited") { - logger.stderr( + stderr( `[StartContainer] Container '${containerName}' exists in state '${existingContainer.State}'. Starting it...`, ); await dockerContainer.start(); @@ -278,7 +278,7 @@ async function startContainer({ containerName, }); } else if (state === "running") { - logger.stderr( + stderr( `[StartContainer] Container '${containerName}' is already running.`, ); } else { @@ -287,7 +287,7 @@ async function startContainer({ ); } } else { - logger.stderr(`[StartContainer] Starting container '${containerName}'...`); + stderr(`[StartContainer] Starting container '${containerName}'...`); const dockerContainer = await createContainer({ imageName, containerName, @@ -312,7 +312,6 @@ async function startContainer({ * @returns {Promise} The log stream */ async function createLogStream({ dockerContainer, containerName }) { - const logger = container.resolve("logger"); let logStream = await dockerContainer.logs({ stdout: true, stderr: true, @@ -322,13 +321,11 @@ async function createLogStream({ dockerContainer, containerName }) { // Pipe the logs to your logger logStream.on("data", (chunk) => { - logger.stderr(`[StartContainer][${containerName}] ${chunk.toString()}`); + stderr(`[StartContainer][${containerName}] ${chunk.toString()}`); }); logStream.on("end", async () => { - logger.stderr( - `[StartContainer] Container '${containerName}' logs have finished.`, - ); + stderr(`[StartContainer] Container '${containerName}' logs have finished.`); logStream = await createLogStream({ dockerContainer, containerName, @@ -336,7 +333,7 @@ async function createLogStream({ dockerContainer, containerName }) { }); logStream.on("error", (error) => { - logger.stderr( + stderr( `[StartContainer] Error tailing logs for container '${containerName}': ${error.message}`, ); }); @@ -360,9 +357,8 @@ async function waitForHealthCheck({ interval = 10000, logStream, }) { - const logger = container.resolve("logger"); const fetch = container.resolve("fetch"); - logger.stderr(`[HealthCheck] Waiting for Fauna to be ready at ${url}...`); + stderr(`[HealthCheck] Waiting for Fauna to be ready at ${url}...`); let attemptCounter = 0; let errorMessage = ""; @@ -374,7 +370,7 @@ async function waitForHealthCheck({ timeout: 1000, }); if (response.ok) { - logger.stderr(`[HealthCheck] Fauna is ready at ${url}`); + stderr(`[HealthCheck] Fauna is ready at ${url}`); logStream?.destroy(); return; } @@ -382,7 +378,7 @@ async function waitForHealthCheck({ } catch (e) { errorMessage = `with error: ${e.message}`; } - logger.stderr( + stderr( `[HealthCheck] Fauna is not yet ready. Attempt ${attemptCounter + 1}/${maxAttempts} failed ${errorMessage}. Retrying in ${interval / 1000} seconds...`, ); attemptCounter++; @@ -392,10 +388,19 @@ async function waitForHealthCheck({ }); } - logger.stderr( + stderr( `[HealthCheck] Max attempts reached. Service at ${url} did not respond.`, ); throw new CommandError( `[HealthCheck] Fauna at ${url} is not ready after ${maxAttempts} attempts. Consider increasing --interval or --maxAttempts.`, ); } + +/** + * Outputs to stderr. + * @param {string} log The log + */ +function stderr(log) { + const logger = container.resolve("logger"); + logger.stderr(colorize(log, { format: Format.LOG, color })); +} diff --git a/src/lib/formatting/codeToAnsi.mjs b/src/lib/formatting/codeToAnsi.mjs index de53f74f..91b7c196 100644 --- a/src/lib/formatting/codeToAnsi.mjs +++ b/src/lib/formatting/codeToAnsi.mjs @@ -2,6 +2,7 @@ import chalk from "chalk"; import { createHighlighterCoreSync } from "shiki/core"; import { createJavaScriptRegexEngine } from "shiki/engine/javascript"; import json from "shiki/langs/json.mjs"; +import log from "shiki/langs/log.mjs"; import githubDarkHighContrast from "shiki/themes/github-dark-high-contrast.mjs"; import { isTTY } from "../misc.mjs"; @@ -12,7 +13,7 @@ const THEME = "github-dark-high-contrast"; export const createHighlighter = () => { const highlighter = createHighlighterCoreSync({ themes: [githubDarkHighContrast], - langs: [json, fql], + langs: [fql, log, json], engine: createJavaScriptRegexEngine(), }); @@ -64,7 +65,7 @@ const { codeToTokensBase, getTheme } = createHighlighter(); * Returns a string with ANSI codes applied to the code. This is a JS port of the * TypeScript codeToAnsi function from the Shiki library. * @param {*} code - The code to format. - * @param {"json" | "fql"} language - The language of the code. + * @param {"fql" | "log" | "json"} language - The language of the code. * @returns {string} - The formatted code with ANSI codes applied. */ export function codeToAnsi(code, language) { diff --git a/src/lib/formatting/colorize.mjs b/src/lib/formatting/colorize.mjs index 30fb1809..a162cc54 100644 --- a/src/lib/formatting/colorize.mjs +++ b/src/lib/formatting/colorize.mjs @@ -1,9 +1,11 @@ import stripAnsi from "strip-ansi"; import { container } from "../../cli.mjs"; +import { codeToAnsi } from "./codeToAnsi.mjs"; export const Format = { FQL: "fql", + LOG: "log", JSON: "json", TEXT: "text", }; @@ -42,6 +44,14 @@ const jsonToAnsi = (obj) => { return res.trim(); }; +const logToAnsi = (obj) => { + if (typeof obj !== "string") { + throw new Error("Unable to format LOG unless it is already a string."); + } + const res = codeToAnsi(obj, "log"); + return res.trim(); +}; + /** * Formats an object for display with ANSI color codes. * @param {any} obj - The object to format @@ -55,6 +65,8 @@ export const toAnsi = (obj, { format = Format.TEXT } = {}) => { return fqlToAnsi(obj); case Format.JSON: return jsonToAnsi(obj); + case Format.LOG: + return logToAnsi(obj); default: return textToAnsi(obj); } diff --git a/test/local.mjs b/test/local.mjs index 72f22a04..66c7abc4 100644 --- a/test/local.mjs +++ b/test/local.mjs @@ -87,7 +87,7 @@ describe("ensureContainerRunning", () => { docker.listContainers.onCall(0).resolves([]); try { // Run the actual command - await run("local", container); + await run("local --no-color", container); throw new Error("Expected an error to be thrown."); } catch (_) { // Expected error, no action needed @@ -120,7 +120,7 @@ Please pass a --hostPort other than '8443'.", logs: logsStub, unpause: unpauseStub, }); - await run("local", container); + await run("local --no-color", container); expect(unpauseStub).not.to.have.been.called; expect(startStub).to.have.been.called; expect(logsStub).to.have.been.calledWith({ @@ -169,7 +169,7 @@ Please pass a --hostPort other than '8443'.", unpause: unpauseStub, }); await run( - "local --hostPort 10 --containerPort 11 --name Taco --hostIp 127.0.0.1", + "local --no-color --hostPort 10 --containerPort 11 --name Taco --hostIp 127.0.0.1", container, ); expect(docker.createContainer).to.have.been.calledWith({ @@ -204,7 +204,7 @@ Please pass a --hostPort other than '8443'.", logs: logsStub, unpause: unpauseStub, }); - await run("local --pull false", container); + await run("local --no-color --pull false", container); expect(docker.pull).not.to.have.been.called; expect(docker.modem.followProgress).not.to.have.been.called; expect(startStub).to.have.been.called; @@ -229,7 +229,7 @@ Please pass a --hostPort other than '8443'.", unpause: unpauseStub, }); try { - await run("local", container); + await run("local --no-color", container); throw new Error("Expected an error to be thrown."); } catch (_) {} expect(docker.pull).to.have.been.called; @@ -271,7 +271,7 @@ https://support.fauna.com/hc/en-us/requests/new`, fetch.onCall(0).rejects(); fetch.resolves(f({}, 503)); // fail from http try { - await run("local --interval 0 --maxAttempts 3", container); + await run("local --no-color --interval 0 --maxAttempts 3", container); throw new Error("Expected an error to be thrown."); } catch (_) {} const written = stderrStream.getWritten(); @@ -307,7 +307,7 @@ https://support.fauna.com/hc/en-us/requests/new`, unpause: unpauseStub, }); try { - await run("local", container); + await run("local --no-color", container); throw new Error("Expected an error to be thrown."); } catch (_) {} const written = stderrStream.getWritten(); @@ -319,7 +319,7 @@ https://support.fauna.com/hc/en-us/requests/new`, it("throws an error if interval is less than 0", async () => { try { - await run("local --interval -1", container); + await run("local --no-color --interval -1", container); throw new Error("Expected an error to be thrown."); } catch (_) {} const written = stderrStream.getWritten(); @@ -332,7 +332,7 @@ https://support.fauna.com/hc/en-us/requests/new`, it("throws an error if maxAttempts is less than 1", async () => { try { - await run("local --maxAttempts 0", container); + await run("local --no-color --maxAttempts 0", container); throw new Error("Expected an error to be thrown."); } catch (_) {} const written = stderrStream.getWritten(); @@ -417,7 +417,7 @@ https://support.fauna.com/hc/en-us/requests/new`, unpause: unpauseStub, }); try { - await run("local", container); + await run("local --no-color", container); } catch (_) { expect(test.state).to.equal("dead"); } @@ -433,7 +433,7 @@ https://support.fauna.com/hc/en-us/requests/new`, test.expectCalls(); expect(logger.stderr).to.have.been.calledWith(test.startMessage); expect(logger.stderr).to.have.been.calledWith( - `[PullImage] Pulling image 'fauna/faunadb:latest'...\n`, + `[PullImage] Pulling image 'fauna/faunadb:latest'...`, ); expect(logger.stderr).to.have.been.calledWith( "[PullImage] Image 'fauna/faunadb:latest' pulled.",