From a36149c48dc1a56e3822383e28bd004da8109f0e Mon Sep 17 00:00:00 2001 From: Sarah Shader Date: Tue, 10 Sep 2024 15:38:08 -0400 Subject: [PATCH] Keep track of CLI clean up functions on context (#29273) I noticed a couple bugs while playing around locally: * If `npx convex dev --local` errored, it wouldn't clean up the local backend process * If I did `npx convex dev --local` for project A, killed it, did `npx convex dev --local` for project B, then went back to A, it would error because it thought a deployment for project A was still running on the old ports. The big change here is keeping track of cleanup functions on `ctx`, which also meant making the `oneoffCtx` into a class instead of a singleton. The cleanup handlers get run on `flushAndExit` (which happens when we error or otherwise force exit the process), and also on SIGINT for `npx convex dev` GitOrigin-RevId: d6701a88a1eaeeb7d1ff90a541e0f17547bff65a --- scripts/bundle-server.mjs | 17 ++----- src/bundler/context.ts | 45 ++++++++++++++----- src/bundler/index.test.ts | 24 +++------- src/cli/auth.ts | 6 +-- src/cli/codegen.ts | 2 +- src/cli/configure.ts | 8 +--- src/cli/convexExport.ts | 2 +- src/cli/convexImport.ts | 2 +- src/cli/dashboard.ts | 2 +- src/cli/data.ts | 2 +- src/cli/deploy.ts | 8 ++-- src/cli/deployments.ts | 2 +- src/cli/dev.ts | 26 +++-------- src/cli/docs.ts | 2 +- src/cli/env.ts | 8 ++-- src/cli/functionSpec.ts | 2 +- src/cli/init.ts | 2 +- src/cli/lib/config.test.ts | 5 ++- src/cli/lib/deployment.ts | 1 - src/cli/lib/fsUtils.test.ts | 2 +- .../lib/localDeployment/localDeployment.ts | 32 +++++++------ src/cli/lib/localDeployment/run.ts | 36 ++++++++++++--- src/cli/lib/localDeployment/upgrade.ts | 16 +++++-- src/cli/lib/push.ts | 2 - src/cli/lib/watch.ts | 32 +++++++------ src/cli/login.ts | 2 +- src/cli/logout.ts | 2 +- src/cli/logs.ts | 2 +- src/cli/network_test.ts | 2 +- src/cli/reinit.ts | 2 +- src/cli/run.ts | 3 +- src/cli/typecheck.ts | 2 +- src/cli/update.ts | 2 +- 33 files changed, 164 insertions(+), 139 deletions(-) diff --git a/scripts/bundle-server.mjs b/scripts/bundle-server.mjs index 7c57a74..015c8f8 100644 --- a/scripts/bundle-server.mjs +++ b/scripts/bundle-server.mjs @@ -14,8 +14,9 @@ const out = []; // Only bundle "setup.ts" from `udf/_system`. const udfDir = process.argv[2]; const setupPath = path.join(udfDir, "setup.ts"); +const ctx = oneoffContext(); const setupBundles = ( - await bundle(oneoffContext, process.argv[2], [setupPath], true, "browser") + await bundle(ctx, process.argv[2], [setupPath], true, "browser") ).modules; if (setupBundles.length !== 1) { throw new Error("Got more than one setup bundle?"); @@ -26,19 +27,9 @@ for (const systemDir of systemDirs) { if (path.basename(systemDir) !== "_system") { throw new Error(`Refusing to bundle non-system directory ${systemDir}`); } - const entryPoints = await entryPointsByEnvironment( - oneoffContext, - systemDir, - false, - ); + const entryPoints = await entryPointsByEnvironment(ctx, systemDir, false); const bundles = ( - await bundle( - oneoffContext, - systemDir, - entryPoints.isolate, - false, - "browser", - ) + await bundle(ctx, systemDir, entryPoints.isolate, false, "browser") ).modules; out.push(...bundles); } diff --git a/src/bundler/context.ts b/src/bundler/context.ts index e92c521..a8aca04 100644 --- a/src/bundler/context.ts +++ b/src/bundler/context.ts @@ -45,6 +45,8 @@ export interface Context { errForSentry?: any; printedMessage: string | null; }): Promise; + registerCleanup(fn: () => Promise): string; + removeCleanup(handle: string): (() => Promise) | null; } async function flushAndExit(exitCode: number, err?: any) { @@ -64,24 +66,45 @@ export type OneoffCtx = Context & { flushAndExit: (exitCode: number, err?: any) => Promise; }; -export const oneoffContext: OneoffCtx = { - fs: nodeFs, - deprecationMessagePrinted: false, - spinner: undefined, - async crash(args: { +class OneoffContextImpl { + private _cleanupFns: Record Promise> = {}; + public fs: Filesystem = nodeFs; + public deprecationMessagePrinted: boolean = false; + public spinner: Ora | undefined = undefined; + crash = async (args: { exitCode: number; errorType?: ErrorType; errForSentry?: any; printedMessage: string | null; - }) { + }) => { if (args.printedMessage !== null) { - logFailure(oneoffContext, args.printedMessage); + logFailure(this, args.printedMessage); } - return await flushAndExit(args.exitCode, args.errForSentry); - }, - flushAndExit, -}; + return await this.flushAndExit(args.exitCode, args.errForSentry); + }; + flushAndExit = async (exitCode: number, err?: any) => { + logVerbose(this, "Flushing and exiting"); + const fns = Object.values(this._cleanupFns); + logVerbose(this, `Running ${fns.length} cleanup functions`); + for (const fn of fns) { + await fn(); + } + logVerbose(this, "All cleanup functions ran"); + return flushAndExit(exitCode, err); + }; + registerCleanup(fn: () => Promise) { + const handle = Math.random().toString(36).slice(2); + this._cleanupFns[handle] = fn; + return handle; + } + removeCleanup(handle: string) { + const value = this._cleanupFns[handle]; + delete this._cleanupFns[handle]; + return value ?? null; + } +} +export const oneoffContext: () => OneoffCtx = () => new OneoffContextImpl(); // console.error before it started being red by default in Node v20 function logToStderr(...args: unknown[]) { process.stderr.write(`${format(...args)}\n`); diff --git a/src/bundler/index.test.ts b/src/bundler/index.test.ts index a5bc543..ce17fe4 100644 --- a/src/bundler/index.test.ts +++ b/src/bundler/index.test.ts @@ -35,21 +35,11 @@ test("bundle function is present", () => { test("bundle finds JavaScript functions", async () => { const fixtureDir = dirname + "/test_fixtures/js/project01"; - const entryPoints = await entryPointsByEnvironment( - oneoffContext, - fixtureDir, - false, - ); + const ctx = oneoffContext(); + const entryPoints = await entryPointsByEnvironment(ctx, fixtureDir, false); const bundles = sorted( - ( - await bundle( - oneoffContext, - fixtureDir, - entryPoints.isolate, - false, - "browser", - ) - ).modules, + (await bundle(ctx, fixtureDir, entryPoints.isolate, false, "browser")) + .modules, (b) => b.path, ).filter((bundle) => !bundle.path.includes("_deps")); expect(bundles).toHaveLength(2); @@ -116,7 +106,7 @@ test("returns true when multiple imports and httpRouter is imported", async () = test("bundle warns about https.js|ts at top level", async () => { const fixtureDir = dirname + "/test_fixtures/js/project_with_https"; const logSpy = vi.spyOn(process.stderr, "write"); - await entryPoints(oneoffContext, fixtureDir, false); + await entryPoints(oneoffContext(), fixtureDir, false); expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("https")); }); @@ -124,7 +114,7 @@ test("bundle does not warn about https.js|ts which is not at top level", async ( const fixtureDir = dirname + "/test_fixtures/js/project_with_https_not_at_top_level"; const logSpy = vi.spyOn(process.stderr, "write"); - await entryPoints(oneoffContext, fixtureDir, false); + await entryPoints(oneoffContext(), fixtureDir, false); expect(logSpy).toHaveBeenCalledTimes(0); }); @@ -132,7 +122,7 @@ test("bundle does not warn about https.js|ts which does not import httpRouter", const fixtureDir = dirname + "/test_fixtures/js/project_with_https_without_router"; const logSpy = vi.spyOn(process.stderr, "write"); - await entryPoints(oneoffContext, fixtureDir, false); + await entryPoints(oneoffContext(), fixtureDir, false); expect(logSpy).toHaveBeenCalledTimes(0); }); diff --git a/src/cli/auth.ts b/src/cli/auth.ts index 0af0c90..fff06c0 100644 --- a/src/cli/auth.ts +++ b/src/cli/auth.ts @@ -2,7 +2,7 @@ import { Command, Option } from "@commander-js/extra-typings"; import { oneoffContext } from "../bundler/context.js"; const list = new Command("list").action(async () => { - const ctx = oneoffContext; + const ctx = oneoffContext(); await ctx.crash({ exitCode: 1, errorType: "fatal", @@ -13,7 +13,7 @@ const list = new Command("list").action(async () => { }); const rm = new Command("remove").action(async () => { - const ctx = oneoffContext; + const ctx = oneoffContext(); await ctx.crash({ exitCode: 1, errorType: "fatal", @@ -27,7 +27,7 @@ const add = new Command("add") .addOption(new Option("--identity-provider-url ").hideHelp()) .addOption(new Option("--application-id ").hideHelp()) .action(async () => { - const ctx = oneoffContext; + const ctx = oneoffContext(); await ctx.crash({ exitCode: 1, errorType: "fatal", diff --git a/src/cli/codegen.ts b/src/cli/codegen.ts index 4d18777..bbca61b 100644 --- a/src/cli/codegen.ts +++ b/src/cli/codegen.ts @@ -35,7 +35,7 @@ export const codegen = new Command("codegen") ).hideHelp(), ) .action(async (options) => { - const ctx = oneoffContext; + const ctx = oneoffContext(); await runCodegen(ctx, { dryRun: !!options.dryRun, diff --git a/src/cli/configure.ts b/src/cli/configure.ts index 5d07351..2a6796d 100644 --- a/src/cli/configure.ts +++ b/src/cli/configure.ts @@ -47,7 +47,6 @@ import { promptOptions, promptString } from "./lib/utils/prompts.js"; type DeploymentCredentials = { url: string; adminKey: string; - cleanupHandle: (() => Promise) | null; }; /** @@ -79,7 +78,6 @@ export async function deploymentCredentialsOrConfigure( ): Promise< DeploymentCredentials & { deploymentName?: DeploymentName; - cleanupHandle: null | (() => Promise); } > { if (cmdOptions.url !== undefined && cmdOptions.adminKey !== undefined) { @@ -87,7 +85,7 @@ export async function deploymentCredentialsOrConfigure( url: cmdOptions.url, adminKey: cmdOptions.adminKey, }); - return { ...credentials, cleanupHandle: null }; + return { ...credentials }; } const { projectSlug, teamSlug } = await selectProject( ctx, @@ -103,7 +101,6 @@ export async function deploymentCredentialsOrConfigure( deploymentName, deploymentUrl: url, adminKey, - cleanupHandle, } = await ensureDeploymentProvisioned(ctx, { teamSlug, projectSlug, @@ -117,7 +114,7 @@ export async function deploymentCredentialsOrConfigure( deploymentType: deploymentOptions.kind, }); - return { deploymentName, url, adminKey, cleanupHandle }; + return { deploymentName, url, adminKey }; } async function handleManuallySetUrlAndAdminKey( @@ -429,7 +426,6 @@ async function ensureDeploymentProvisioned( ); return { ...credentials, - cleanupHandle: null, onActivity: null, }; } diff --git a/src/cli/convexExport.ts b/src/cli/convexExport.ts index f612ba3..93bc717 100644 --- a/src/cli/convexExport.ts +++ b/src/cli/convexExport.ts @@ -46,7 +46,7 @@ export const convexExport = new Command("export") .addDeploymentSelectionOptions(actionDescription("Export data from")) .showHelpAfterError() .action(async (options) => { - const ctx = oneoffContext; + const ctx = oneoffContext(); const deploymentSelection = deploymentSelectionFromOptions(options); diff --git a/src/cli/convexImport.ts b/src/cli/convexImport.ts index 45fea18..fd150e3 100644 --- a/src/cli/convexImport.ts +++ b/src/cli/convexImport.ts @@ -86,7 +86,7 @@ export const convexImport = new Command("import") .argument("", "Path to the input file") .showHelpAfterError() .action(async (filePath, options, command) => { - const ctx = oneoffContext; + const ctx = oneoffContext(); if (command.args.length > 1) { return await ctx.crash({ diff --git a/src/cli/dashboard.ts b/src/cli/dashboard.ts index 1dd3f27..7cb3c81 100644 --- a/src/cli/dashboard.ts +++ b/src/cli/dashboard.ts @@ -21,7 +21,7 @@ export const dashboard = new Command("dashboard") .addDeploymentSelectionOptions(actionDescription("Open the dashboard for")) .showHelpAfterError() .action(async (options) => { - const ctx = oneoffContext; + const ctx = oneoffContext(); const deploymentSelection = deploymentSelectionFromOptions(options); const { deploymentName } = await fetchDeploymentCredentialsProvisionProd( diff --git a/src/cli/data.ts b/src/cli/data.ts index fad4525..bc4c41c 100644 --- a/src/cli/data.ts +++ b/src/cli/data.ts @@ -54,7 +54,7 @@ export const data = new Command("data") .addDeploymentSelectionOptions(actionDescription("Inspect the database in")) .showHelpAfterError() .action(async (tableName, options) => { - const ctx = oneoffContext; + const ctx = oneoffContext(); const deploymentSelection = deploymentSelectionFromOptions(options); const { diff --git a/src/cli/deploy.ts b/src/cli/deploy.ts index 1821f3c..a3a0415 100644 --- a/src/cli/deploy.ts +++ b/src/cli/deploy.ts @@ -114,7 +114,7 @@ export const deploy = new Command("deploy") .addOption(new Option("--live-component-sources").hideHelp()) .showHelpAfterError() .action(async (cmdOptions) => { - const ctx = oneoffContext; + const ctx = oneoffContext(); storeAdminKeyEnvVar(cmdOptions.adminKey); const configuredDeployKey = readAdminKeyFromEnvVar() ?? null; @@ -232,11 +232,10 @@ async function deployToNewPreviewDeployment( debugBundlePath: options.debugBundlePath, codegen: options.codegen === "enable", url: previewUrl, - cleanupHandle: null, liveComponentSources: false, }; showSpinner(ctx, `Deploying to ${previewUrl}...`); - await runPush(oneoffContext, pushOptions); + await runPush(ctx, pushOptions); logFinishedStep(ctx, `Deployed Convex functions to ${previewUrl}`); if (options.previewRun !== undefined) { @@ -329,14 +328,13 @@ async function deployToExistingDeployment( codegen: options.codegen === "enable", url, writePushRequest: options.writePushRequest, - cleanupHandle: null, liveComponentSources: !!options.liveComponentSources, }; showSpinner( ctx, `Deploying to ${url}...${options.dryRun ? " [dry run]" : ""}`, ); - await runPush(oneoffContext, pushOptions); + await runPush(ctx, pushOptions); logFinishedStep( ctx, `${ diff --git a/src/cli/deployments.ts b/src/cli/deployments.ts index 41423dc..8690297 100644 --- a/src/cli/deployments.ts +++ b/src/cli/deployments.ts @@ -14,7 +14,7 @@ type Deployment = { export const deployments = new Command("deployments") .description("List deployments associated with a project") .action(async () => { - const ctx = oneoffContext; + const ctx = oneoffContext(); const { projectConfig: config } = await readProjectConfig(ctx); const url = `teams/${config.team}/projects/${config.project}/deployments`; diff --git a/src/cli/dev.ts b/src/cli/dev.ts index 7e2da6b..0c1a2a9 100644 --- a/src/cli/dev.ts +++ b/src/cli/dev.ts @@ -115,7 +115,11 @@ export const dev = new Command("dev") .addOption(new Option("--live-component-sources").hideHelp()) .showHelpAfterError() .action(async (cmdOptions) => { - const ctx = oneoffContext; + const ctx = oneoffContext(); + process.on("SIGINT", async () => { + logVerbose(ctx, "Received SIGINT, cleaning up..."); + await ctx.flushAndExit(-2); + }); if (cmdOptions.debugBundlePath !== undefined && !cmdOptions.once) { return await ctx.crash({ @@ -175,19 +179,6 @@ export const dev = new Command("dev") ...cmdOptions, localOptions, }); - let cleanupHandle = credentials.cleanupHandle; - process.on("SIGINT", async () => { - logVerbose(ctx, "Received SIGINT, cleaning up..."); - if (cleanupHandle !== null) { - logVerbose(ctx, "About to run cleanup handle."); - // Sometimes `SIGINT` gets sent twice, so set `cleanupHandle` to null to prevent double-cleanup - const f = cleanupHandle; - cleanupHandle = null; - await f(); - } - logVerbose(ctx, "Cleaned up. Exiting."); - process.exit(-2); - }); await usageStateWarning(ctx); @@ -214,6 +205,7 @@ export const dev = new Command("dev") ), ); await Promise.race(promises); + await ctx.flushAndExit(0); }); export async function watchAndPush( @@ -296,15 +288,9 @@ export async function watchAndPush( stopSpinner(ctx); } if (cmdOptions.once) { - if (options.cleanupHandle !== null) { - await options.cleanupHandle(); - } return; } if (pushed && cmdOptions.untilSuccess) { - if (options.cleanupHandle !== null) { - await options.cleanupHandle(); - } return; } const fileSystemWatch = getFileSystemWatch(ctx, watch, cmdOptions); diff --git a/src/cli/docs.ts b/src/cli/docs.ts index f57d73c..5a00046 100644 --- a/src/cli/docs.ts +++ b/src/cli/docs.ts @@ -9,7 +9,7 @@ export const docs = new Command("docs") .description("Open the docs in the browser") .option("--no-open", "Print docs URL instead of opening it in your browser") .action(async (options) => { - const ctx = oneoffContext; + const ctx = oneoffContext(); // Usually we'd call `getConfiguredDeploymentName` but in this // command we don't care at all if the user is in the right directory const configuredDeployment = getTargetDeploymentName(); diff --git a/src/cli/env.ts b/src/cli/env.ts index 986b21e..31e1a19 100644 --- a/src/cli/env.ts +++ b/src/cli/env.ts @@ -35,7 +35,7 @@ const envSet = new Command("set") .allowExcessArguments(false) .action(async (originalName, originalValue, _options, cmd) => { const options = cmd.optsWithGlobals(); - const ctx = oneoffContext; + const ctx = oneoffContext(); await ensureHasConvexDependency(ctx, "env set"); const [name, value] = await allowEqualsSyntax( ctx, @@ -78,7 +78,7 @@ const envGet = new Command("get") .configureHelp({ showGlobalOptions: true }) .allowExcessArguments(false) .action(async (envVarName, _options, cmd) => { - const ctx = oneoffContext; + const ctx = oneoffContext(); await ensureHasConvexDependency(ctx, "env get"); const options = cmd.optsWithGlobals(); const deploymentSelection = deploymentSelectionFromOptions(options); @@ -116,7 +116,7 @@ const envRemove = new Command("remove") .configureHelp({ showGlobalOptions: true }) .allowExcessArguments(false) .action(async (name, _options, cmd) => { - const ctx = oneoffContext; + const ctx = oneoffContext(); const options = cmd.optsWithGlobals(); await ensureHasConvexDependency(ctx, "env remove"); const where = await callUpdateEnvironmentVariables(ctx, options, [ @@ -131,7 +131,7 @@ const envList = new Command("list") .configureHelp({ showGlobalOptions: true }) .allowExcessArguments(false) .action(async (_options, cmd) => { - const ctx = oneoffContext; + const ctx = oneoffContext(); await ensureHasConvexDependency(ctx, "env list"); const options = cmd.optsWithGlobals(); const deploymentSelection = deploymentSelectionFromOptions(options); diff --git a/src/cli/functionSpec.ts b/src/cli/functionSpec.ts index ea5b7b4..21bac98 100644 --- a/src/cli/functionSpec.ts +++ b/src/cli/functionSpec.ts @@ -20,7 +20,7 @@ export const functionSpec = new Command("function-spec") ) .showHelpAfterError() .action(async (options) => { - const ctx = oneoffContext; + const ctx = oneoffContext(); const deploymentSelection = deploymentSelectionFromOptions(options); const { adminKey, url: deploymentUrl } = diff --git a/src/cli/init.ts b/src/cli/init.ts index 5728f69..01e3232 100644 --- a/src/cli/init.ts +++ b/src/cli/init.ts @@ -22,7 +22,7 @@ export const init = new Command("init") ), ) .action(async (_options) => { - return oneoffContext.crash({ + return oneoffContext().crash({ exitCode: 1, errorType: "fatal", errForSentry: diff --git a/src/cli/lib/config.test.ts b/src/cli/lib/config.test.ts index 1d1deaf..154d942 100644 --- a/src/cli/lib/config.test.ts +++ b/src/cli/lib/config.test.ts @@ -5,11 +5,12 @@ import stripAnsi from "strip-ansi"; test("parseProjectConfig", async () => { // Make a context that throws on crashes so we can detect them. + const originalContext = oneoffContext(); const ctx = { - ...oneoffContext, + ...originalContext, crash: (args: { printedMessage: string | null }) => { if (args.printedMessage !== null) { - logFailure(oneoffContext, args.printedMessage); + logFailure(originalContext, args.printedMessage); } throw new Error(); }, diff --git a/src/cli/lib/deployment.ts b/src/cli/lib/deployment.ts index d5affaf..56cd68d 100644 --- a/src/cli/lib/deployment.ts +++ b/src/cli/lib/deployment.ts @@ -251,6 +251,5 @@ export type DeploymentDetails = { deploymentName: string; deploymentUrl: string; adminKey: string; - cleanupHandle: CleanupDeploymentFunc | null; onActivity: OnDeploymentActivityFunc | null; }; diff --git a/src/cli/lib/fsUtils.test.ts b/src/cli/lib/fsUtils.test.ts index 5fc2414..f758d00 100644 --- a/src/cli/lib/fsUtils.test.ts +++ b/src/cli/lib/fsUtils.test.ts @@ -7,7 +7,7 @@ import { recursivelyDelete } from "./fsUtils.js"; describe("fsUtils", () => { let tmpDir: string; - const ctx = oneoffContext; + const ctx = oneoffContext(); beforeEach(() => { tmpDir = fs.mkdtempSync(`${os.tmpdir()}${path.sep}`); diff --git a/src/cli/lib/localDeployment/localDeployment.ts b/src/cli/lib/localDeployment/localDeployment.ts index bc18e42..1e1a236 100644 --- a/src/cli/lib/localDeployment/localDeployment.ts +++ b/src/cli/lib/localDeployment/localDeployment.ts @@ -19,17 +19,13 @@ import { runLocalBackend, } from "./run.js"; import { handlePotentialUpgrade } from "./upgrade.js"; -import { - CleanupDeploymentFunc, - OnDeploymentActivityFunc, -} from "../deployment.js"; +import { OnDeploymentActivityFunc } from "../deployment.js"; import { promptSearch } from "../utils/prompts.js"; export type DeploymentDetails = { deploymentName: string; deploymentUrl: string; adminKey: string; - cleanupHandle: CleanupDeploymentFunc; onActivity: OnDeploymentActivityFunc; }; @@ -59,11 +55,16 @@ export async function handleLocalDeployment( ctx, `Found existing deployment for project ${options.projectSlug}`, ); + // If it's still running for some reason, exit and tell the user to kill it. + // It's fine if a different backend is running on these ports though since we'll + // pick new ones. await ensureBackendStopped(ctx, { ports: { cloud: existingDeploymentForProject.config.ports.cloud, }, maxTimeSecs: 5, + deploymentName: existingDeploymentForProject.deploymentName, + allowOtherDeployments: true, }); } @@ -106,17 +107,21 @@ export async function handleLocalDeployment( forceUpgrade: options.forceUpgrade, }); + const cleanupFunc = ctx.removeCleanup(cleanupHandle); + ctx.registerCleanup(async () => { + if (cleanupFunc !== null) { + await cleanupFunc(); + } + await bigBrainPause(ctx, { + projectSlug: options.projectSlug, + teamSlug: options.teamSlug, + }); + }); + return { adminKey, deploymentName, deploymentUrl: localDeploymentUrl(ports.cloud), - cleanupHandle: async () => { - await cleanupHandle(); - await bigBrainPause(ctx, { - projectSlug: options.projectSlug, - teamSlug: options.teamSlug, - }); - }, onActivity, }; } @@ -137,7 +142,7 @@ async function handleOffline( }); const ports = await choosePorts(ctx, options.ports); saveDeploymentConfig(ctx, deploymentName, config); - const { cleanupHandle } = await runLocalBackend(ctx, { + await runLocalBackend(ctx, { binaryPath, ports, deploymentName, @@ -146,7 +151,6 @@ async function handleOffline( adminKey: config.adminKey, deploymentName, deploymentUrl: localDeploymentUrl(ports.cloud), - cleanupHandle, onActivity: async (isOffline: boolean, wasOffline: boolean) => { await ensureBackendRunning(ctx, { cloudPort: ports.cloud, diff --git a/src/cli/lib/localDeployment/run.ts b/src/cli/lib/localDeployment/run.ts index 91a7a4e..c24e3e3 100644 --- a/src/cli/lib/localDeployment/run.ts +++ b/src/cli/lib/localDeployment/run.ts @@ -132,7 +132,7 @@ export async function runLocalBackend( binaryPath: string; }, ): Promise<{ - cleanupHandle: () => Promise; + cleanupHandle: string; }> { const { ports } = args; const deploymentDir = deploymentStateDir(args.deploymentName); @@ -160,6 +160,10 @@ export async function runLocalBackend( `Local backend exited with code ${code}, full command \`${commandStr}\``, ); }); + const cleanupHandle = ctx.registerCleanup(async () => { + logVerbose(ctx, `Stopping local backend on port ${ports.cloud}`); + p.kill("SIGTERM"); + }); await ensureBackendRunning(ctx, { cloudPort: ports.cloud, @@ -168,10 +172,7 @@ export async function runLocalBackend( }); return { - cleanupHandle: async () => { - logVerbose(ctx, `Stopping local backend on port ${ports.cloud}`); - p.kill("SIGTERM"); - }, + cleanupHandle, }; } @@ -221,6 +222,9 @@ export async function ensureBackendStopped( site?: number; }; maxTimeSecs: number; + deploymentName: string; + // Whether to allow a deployment with a different name to run on this port + allowOtherDeployments: boolean; }, ) { logVerbose( @@ -236,6 +240,28 @@ export async function ensureBackendStopped( if (cloudPort === args.ports.cloud && sitePort === args.ports.site) { return; } + try { + const instanceNameResp = await fetch( + `${localDeploymentUrl(args.ports.cloud)}/instance_name`, + ); + if (instanceNameResp.ok) { + const instanceName = await instanceNameResp.text(); + if (instanceName !== args.deploymentName) { + if (args.allowOtherDeployments) { + return; + } + return await ctx.crash({ + exitCode: 1, + errorType: "fatal", + printedMessage: `A different local backend ${instanceName} is running on selected port ${args.ports.cloud}`, + }); + } + } + } catch (error: any) { + logVerbose(ctx, `Error checking if backend is running: ${error.message}`); + // Backend is probably not running + continue; + } await new Promise((resolve) => setTimeout(resolve, 500)); timeElapsedSecs += 0.5; } diff --git a/src/cli/lib/localDeployment/upgrade.ts b/src/cli/lib/localDeployment/upgrade.ts index e4393f5..d5e0f03 100644 --- a/src/cli/lib/localDeployment/upgrade.ts +++ b/src/cli/lib/localDeployment/upgrade.ts @@ -40,7 +40,7 @@ export async function handlePotentialUpgrade( adminKey: string; forceUpgrade: boolean; }, -): Promise<{ cleanupHandle: () => Promise }> { +): Promise<{ cleanupHandle: string }> { const newConfig = { ports: args.ports, backendVersion: args.newVersion, @@ -123,7 +123,7 @@ async function handleUpgrade( }; adminKey: string; }, -): Promise<{ cleanupHandle: () => Promise }> { +): Promise<{ cleanupHandle: string }> { const { binaryPath: oldBinaryPath } = await ensureBackendBinaryDownloaded( ctx, { @@ -183,8 +183,16 @@ async function handleUpgrade( }); logVerbose(ctx, "Stopping the backend on the old version"); - await oldCleanupHandle(); - await ensureBackendStopped(ctx, { ports: args.ports, maxTimeSecs: 5 }); + const oldCleanupFunc = ctx.removeCleanup(oldCleanupHandle); + if (oldCleanupFunc) { + await oldCleanupFunc(); + } + await ensureBackendStopped(ctx, { + ports: args.ports, + maxTimeSecs: 5, + deploymentName: args.deploymentName, + allowOtherDeployments: false, + }); // TODO(ENG-7078) save old artifacts to backup files logVerbose(ctx, "Running backend on new version"); diff --git a/src/cli/lib/push.ts b/src/cli/lib/push.ts index a4708f3..af0a629 100644 --- a/src/cli/lib/push.ts +++ b/src/cli/lib/push.ts @@ -12,7 +12,6 @@ import { pushSchema } from "./indexes.js"; import { typeCheckFunctionsInMode } from "./typecheck.js"; import { ensureHasConvexDependency, functionsDir } from "./utils/utils.js"; import { handleDebugBundlePath } from "./debugBundlePath.js"; -import { CleanupDeploymentFunc } from "./deployment.js"; export type PushOptions = { adminKey: string; @@ -24,7 +23,6 @@ export type PushOptions = { codegen: boolean; url: string; writePushRequest?: string; - cleanupHandle: CleanupDeploymentFunc | null; liveComponentSources: boolean; }; diff --git a/src/cli/lib/watch.ts b/src/cli/lib/watch.ts index af0c090..339b870 100644 --- a/src/cli/lib/watch.ts +++ b/src/cli/lib/watch.ts @@ -1,12 +1,7 @@ import chokidar from "chokidar"; import path from "path"; import { Observations, RecordingFs, WatchEvent } from "../../bundler/fs.js"; -import { - Context, - ErrorType, - logFailure, - logWarning, -} from "../../bundler/context.js"; +import { Context, ErrorType, logFailure } from "../../bundler/context.js"; import * as Sentry from "@sentry/node"; import { Ora } from "ora"; @@ -100,6 +95,7 @@ export class Crash extends Error { } export class WatchContext implements Context { + private _cleanupFns: Record Promise> = {}; fs: RecordingFs; deprecationMessagePrinted: boolean; spinner: Ora | undefined; @@ -109,25 +105,35 @@ export class WatchContext implements Context { this.deprecationMessagePrinted = false; } - crash(args: { + async crash(args: { exitCode: number; errorType?: ErrorType; errForSentry?: any; printedMessage: string | null; - messageLevel?: "error" | "warning"; }): Promise { if (args.errForSentry) { Sentry.captureException(args.errForSentry); } if (args.printedMessage !== null) { - if (args.messageLevel === "warning") { - logWarning(this, args.printedMessage); - } else { - logFailure(this, args.printedMessage); - } + logFailure(this, args.printedMessage); + } + for (const fn of Object.values(this._cleanupFns)) { + await fn(); } // Okay to throw here. We've wrapped it in a Crash that we'll catch later. // eslint-disable-next-line no-restricted-syntax throw new Crash(args.errorType, args.errForSentry); } + + registerCleanup(fn: () => Promise): string { + const handle = Math.random().toString(36).slice(2); + this._cleanupFns[handle] = fn; + return handle; + } + + removeCleanup(handle: string) { + const value = this._cleanupFns[handle]; + delete this._cleanupFns[handle]; + return value ?? null; + } } diff --git a/src/cli/login.ts b/src/cli/login.ts index a3b10d0..e33d621 100644 --- a/src/cli/login.ts +++ b/src/cli/login.ts @@ -31,7 +31,7 @@ export const login = new Command("login") // Hidden option for tests to check if the user is logged in. .addOption(new Option("--check-login").hideHelp()) .action(async (options, cmd: Command) => { - const ctx = oneoffContext; + const ctx = oneoffContext(); if ( !options.force && (await checkAuthorization(ctx, !!options.acceptOptIns)) diff --git a/src/cli/logout.ts b/src/cli/logout.ts index fcd6e76..5f63428 100644 --- a/src/cli/logout.ts +++ b/src/cli/logout.ts @@ -6,7 +6,7 @@ import { recursivelyDelete } from "./lib/fsUtils.js"; export const logout = new Command("logout") .description("Log out of Convex on this machine") .action(async () => { - const ctx = oneoffContext; + const ctx = oneoffContext(); recursivelyDelete(ctx, globalConfigPath()); diff --git a/src/cli/logs.ts b/src/cli/logs.ts index f580c8e..f3bdb07 100644 --- a/src/cli/logs.ts +++ b/src/cli/logs.ts @@ -27,7 +27,7 @@ export const logs = new Command("logs") .addDeploymentSelectionOptions(actionDescription("Watch logs from")) .showHelpAfterError() .action(async (cmdOptions) => { - const ctx = oneoffContext; + const ctx = oneoffContext(); const deploymentSelection = deploymentSelectionFromOptions(cmdOptions); const credentials = await fetchDeploymentCredentialsProvisionProd( diff --git a/src/cli/network_test.ts b/src/cli/network_test.ts index 536ee96..df3cc54 100644 --- a/src/cli/network_test.ts +++ b/src/cli/network_test.ts @@ -66,7 +66,7 @@ export const networkTest = new Command("network-test") .addOption(new Option("--url ")) .action(async (options) => { - const ctx = oneoffContext; + const ctx = oneoffContext(); const timeoutSeconds = options.timeout ? Number.parseFloat(options.timeout) : 30; diff --git a/src/cli/reinit.ts b/src/cli/reinit.ts index 3a33c1c..db041c8 100644 --- a/src/cli/reinit.ts +++ b/src/cli/reinit.ts @@ -21,7 +21,7 @@ export const reinit = new Command("reinit") ), ) .action(async (_options) => { - return oneoffContext.crash({ + return oneoffContext().crash({ exitCode: 1, errorType: "fatal", errForSentry: diff --git a/src/cli/run.ts b/src/cli/run.ts index a015596..128ea54 100644 --- a/src/cli/run.ts +++ b/src/cli/run.ts @@ -52,7 +52,7 @@ export const run = new Command("run") .showHelpAfterError() .action(async (functionName, argsString, options) => { - const ctx = oneoffContext; + const ctx = oneoffContext(); const deploymentSelection = deploymentSelectionFromOptions(options); @@ -87,7 +87,6 @@ export const run = new Command("run") debug: false, codegen: options.codegen === "enable", url: deploymentUrl, - cleanupHandle: null, liveComponentSources: !!options.liveComponentSources, }, { diff --git a/src/cli/typecheck.ts b/src/cli/typecheck.ts index 78ee04f..0616c89 100644 --- a/src/cli/typecheck.ts +++ b/src/cli/typecheck.ts @@ -20,7 +20,7 @@ export const typecheck = new Command("typecheck") "Run TypeScript typechecking on your Convex functions with `tsc --noEmit`.", ) .action(async () => { - const ctx = oneoffContext; + const ctx = oneoffContext(); const { configPath, config: localConfig } = await readConfig(ctx, false); await ensureHasConvexDependency(ctx, "typecheck"); await typeCheckFunctions( diff --git a/src/cli/update.ts b/src/cli/update.ts index 252fbd3..34d9c65 100644 --- a/src/cli/update.ts +++ b/src/cli/update.ts @@ -6,7 +6,7 @@ import { loadPackageJson } from "./lib/utils/utils.js"; export const update = new Command("update") .description("Print instructions for updating the convex package") .action(async () => { - const ctx = oneoffContext; + const ctx = oneoffContext(); let updateInstructions = "npm install convex@latest\n"; const packages = await loadPackageJson(ctx); const oldPackageNames = Object.keys(packages).filter((name) =>