sql:migrate --force should ignore invalid connector

src/dataconnect/schemaMigration.ts

@@ -13,7 +13,7 @@
 import { logLabeledBullet, logLabeledWarning, logLabeledSuccess } from "../utils";
 import * as errors from "./errors";
 
 export async function diffSchema(schema: Schema): Promise<Diff[]> {
   const { serviceName, instanceName, databaseId } = getIdentifiers(schema);
   await ensureServiceIsConnectedToCloudSql(
     serviceName,
@@ -24,8 +24,8 @@
   try {
     await upsertSchema(schema, /** validateOnly=*/ true);
     logLabeledSuccess("dataconnect", `Database schema is up to date.`);
   } catch (err: any) {
     if (err.status !== 400) {
       throw err;
     }
     const invalidConnectors = errors.getInvalidConnectors(err);
@@ -47,7 +47,7 @@
   return [];
 }
 
 export async function migrateSchema(args: {
   options: Options;
   schema: Schema;
   allowNonInteractiveMigration: boolean;
@@ -65,8 +65,8 @@
   try {
     await upsertSchema(schema, validateOnly);
     logger.debug(`Database schema was up to date for ${instanceId}:${databaseId}`);
   } catch (err: any) {
     if (err.status !== 400) {
       throw err;
     }
     // Parse and handle failed precondition errors, then retry.
@@ -79,15 +79,11 @@
 
     const shouldDeleteInvalidConnectors = await promptForInvalidConnectorError(
       options,
+      serviceName,
       invalidConnectors,
       validateOnly,
     );
-    if (!shouldDeleteInvalidConnectors && invalidConnectors.length) {
-      const cmd = suggestedCommand(serviceName, invalidConnectors);
-      throw new FirebaseError(
-        `Command aborted. Try deploying those connectors first with ${clc.bold(cmd)}`,
-      );
-    }
+
     const migrationMode = incompatible
       ? await promptForSchemaMigration(
           options,
@@ -112,11 +108,13 @@
       });
     }
 
-    if (invalidConnectors.length) {
+    if (shouldDeleteInvalidConnectors) {
       await deleteInvalidConnectors(invalidConnectors);
     }
-    // Then, try to upsert schema again. If there still is an error, just throw it now
-    await upsertSchema(schema, validateOnly);
+    if (!validateOnly) {
+      // Then, try to upsert schema again. If there still is an error, just throw it now
+      await upsertSchema(schema, validateOnly);
+    }
     return diffs;
   }
   return [];
@@ -140,7 +138,7 @@
       "tried to migrate schema but instance name was not provided in dataconnect.yaml",
     );
   }
   const instanceId = instanceName.split("/").pop()!;
   const serviceName = schema.name.replace(`/schemas/${SCHEMA_ID}`, "");
   return {
     databaseId,
@@ -212,7 +210,7 @@
 ): Promise<"none" | "safe" | "all"> {
   displaySchemaChanges(err);
   if (!options.nonInteractive) {
-    // Always prompt in interactive mode. Desturctive migrations are too potentially dangerous to not prompt for with --force
+    // Always prompt in interactive mode. Destructive migrations are too potentially dangerous to not prompt for with --force
     const choices = err.destructive
       ? [
           { name: "Execute all changes (including destructive changes)", value: "all" },
@@ -222,7 +220,7 @@
           { name: "Execute changes", value: "safe" },
           { name: "Abort changes", value: "none" },
         ];
     return await promptOnce({
       message: `Would you like to execute these changes against ${databaseName}?`,
       type: "list",
       choices,
@@ -250,6 +248,7 @@
 
 async function promptForInvalidConnectorError(
   options: Options,
+  serviceName: string,
   invalidConnectors: string[],
   validateOnly: boolean,
 ): Promise<boolean> {
@@ -258,25 +257,40 @@
   }
   displayInvalidConnectors(invalidConnectors);
   if (validateOnly) {
-    return false;
-  } else if (
-    options.force ||
-    (!options.nonInteractive &&
-      (await confirm({
-        ...options,
-        message: "Would you like to delete and recreate these connectors?",
-      })))
+    if (options.force) {
+      // `firebase dataconnect:sql:migrate --force` ignores invalid connectors.
+      return false;
+    }
+    // `firebase dataconnect:sql:migrate` aborts if there are invalid connectors.
+    throw new FirebaseError(
+      `Command aborted. If you'd like to migrate it anyway, you may override with --force.`,
+    );
+  }
+  if (options.force) {
+    // `firebase deploy --force` will delete invalid connectors without prompting.
+    return true;
+  }
+  // `firebase deploy` prompts in case of invalid connectors.
+  if (
+    !options.nonInteractive &&
+    (await confirm({
+      ...options,
+      message: `Would you like to delete and recreate these connectors? This will cause ${clc.red(`downtime.`)}.`,
+    }))
   ) {
     return true;
   }
-  return false;
+  const cmd = suggestedCommand(serviceName, invalidConnectors);
+  throw new FirebaseError(
+    `Command aborted. Try deploying those connectors first with ${clc.bold(cmd)}`,
+  );
 }
 
 async function deleteInvalidConnectors(invalidConnectors: string[]): Promise<void[]> {
   return Promise.all(invalidConnectors.map(deleteConnector));
 }
 
 function displayInvalidConnectors(invalidConnectors: string[]) {
   const connectorIds = invalidConnectors.map((i) => i.split("/").pop()).join(", ");
   logLabeledWarning(
     "dataconnect",
@@ -284,7 +298,7 @@
   );
   logLabeledWarning(
     "dataconnect",
-    `This is a ${clc.red("breaking")} change and will cause a brief downtime.`,
+    `This is a ${clc.red("breaking")} change and may break existing apps.`,
   );
 }
 
@@ -292,7 +306,7 @@
 // (ie when users create a service in console),
 // the backend will not have the necessary permissions to check cSQL for differences.
 // We fix this by upserting the currently deployed schema with schemaValidation=strict,
 async function ensureServiceIsConnectedToCloudSql(
   serviceName: string,
   instanceId: string,
   databaseId: string,