Error handling and logging when linking CSQL instance

src/dataconnect/client.ts

@@ -5,14 +5,14 @@
 import { logger } from "../logger";
 
 const DATACONNECT_API_VERSION = "v1alpha";
 const dataconnectClient = () =>
   new Client({
     urlPrefix: dataconnectOrigin(),
     apiVersion: DATACONNECT_API_VERSION,
     auth: true,
   });
 
 export async function listLocations(projectId: string): Promise<string[]> {
   const res = await dataconnectClient().get<{
     locations: {
       name: string;
@@ -33,7 +33,7 @@
         const locationServices = await listServices(projectId, l);
         services = services.concat(locationServices);
       } catch (err) {
         logger.debug(`Unable to listServices in ${l}: ${err}`);
       }
     }),
   );
@@ -41,7 +41,7 @@
   return services;
 }
 
 export async function listServices(
   projectId: string,
   locationId: string,
 ): Promise<types.Service[]> {
@@ -51,7 +51,7 @@
   return res.body.services ?? [];
 }
 
 export async function createService(
   projectId: string,
   locationId: string,
   serviceId: string,
@@ -75,13 +75,14 @@
   return pollRes;
 }
 
 export async function deleteService(
   projectId: string,
   locationId: string,
   serviceId: string,
 ): Promise<types.Service> {
+  // NOTE(fredzqm): Don't force delete yet. Backend would leave orphaned resources.
   const op = await dataconnectClient().delete<types.Service>(
-    `projects/${projectId}/locations/${locationId}/services/${serviceId}?force=true`,
+    `projects/${projectId}/locations/${locationId}/services/${serviceId}`,
   );
   const pollRes = await operationPoller.pollOperation<types.Service>({
     apiOrigin: dataconnectOrigin(),
@@ -93,16 +94,16 @@
 
 /** Schema methods */
 
 export async function getSchema(serviceName: string): Promise<types.Schema> {
   const res = await dataconnectClient().get<types.Schema>(
     `${serviceName}/schemas/${types.SCHEMA_ID}`,
   );
   return res.body;
 }
 
 export async function upsertSchema(
   schema: types.Schema,
   validateOnly: boolean = false,
 ): Promise<types.Schema | undefined> {
   const op = await dataconnectClient().patch<types.Schema, types.Schema>(`${schema.name}`, schema, {
     queryParams: {
@@ -122,7 +123,7 @@
 
 /** Connector methods */
 
 export async function getConnector(name: string): Promise<types.Connector> {
   const res = await dataconnectClient().get<types.Connector>(name);
   return res.body;
 }

src/dataconnect/schemaMigration.ts

@@ -10,26 +10,40 @@ import { Schema } from "./types";
 import { Options } from "../options";
 import { FirebaseError } from "../error";
 import { needProjectId } from "../projectUtils";
-import { logLabeledWarning, logLabeledSuccess } from "../utils";
+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, instanceName, databaseId);
+  await ensureServiceIsConnectedToCloudSql(
+    serviceName,
+    instanceName,
+    databaseId,
+    /* linkIfNotConnected=*/ false,
+  );
   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);
+    const incompatible = errors.getIncompatibleSchemaError(err);
+    if (!incompatible && !invalidConnectors.length) {
+      // If we got a different type of error, throw it
+      throw err;
+    }
+
+    // Display failed precondition errors nicely.
     if (invalidConnectors.length) {
       displayInvalidConnectors(invalidConnectors);
     }
-    const incompatible = errors.getIncompatibleSchemaError(err);
     if (incompatible) {
       displaySchemaChanges(incompatible);
       return incompatible.diffs;
     }
   }
-  logLabeledSuccess("dataconnect", `Database schema is up to date.`);
   return [];
 }
 
@@ -42,17 +56,27 @@ export async function migrateSchema(args: {
   const { options, schema, allowNonInteractiveMigration, validateOnly } = args;
 
   const { serviceName, instanceId, instanceName, databaseId } = getIdentifiers(schema);
-  await ensureServiceIsConnectedToCloudSql(serviceName, instanceName, databaseId);
+  await ensureServiceIsConnectedToCloudSql(
+    serviceName,
+    instanceName,
+    databaseId,
+    /* linkIfNotConnected=*/ true,
+  );
   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.
     const incompatible = errors.getIncompatibleSchemaError(err);
     const invalidConnectors = errors.getInvalidConnectors(err);
     if (!incompatible && !invalidConnectors.length) {
       // If we got a different type of error, throw it
       throw err;
     }
+
     const shouldDeleteInvalidConnectors = await promptForInvalidConnectorError(
       options,
       invalidConnectors,
@@ -266,44 +290,61 @@ function displayInvalidConnectors(invalidConnectors: string[]) {
 
 // If a service has never had a schema with schemaValidation=strict
 // (ie when users create a service in console),
-// the backend will not have the necesary permissions to check cSQL for differences.
+// 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,
+  linkIfNotConnected: boolean,
 ) {
   let currentSchema: Schema;
   try {
     currentSchema = await getSchema(serviceName);
   } catch (err: any) {
-    if (err.status === 404) {
-      // If no schema has been deployed yet, deploy an empty one to get connectivity.
-      currentSchema = {
-        name: `${serviceName}/schemas/${SCHEMA_ID}`,
-        source: {
-          files: [],
-        },
-        primaryDatasource: {
-          postgresql: {
-            database: databaseId,
-            cloudSql: {
-              instance: instanceId,
-            },
-          },
-        },
-      };
-    } else {
+    if (err.status !== 404) {
       throw err;
     }
+    if (!linkIfNotConnected) {
+      logLabeledWarning("dataconnect", `Not yet linked to the Cloud SQL instance.`);
+      return;
+    }
+    // TODO: make this prompt
+    // Should we upsert service here as well? so `database:sql:migrate` work for new service as well.
+    logLabeledBullet("dataconnect", `Linking the Cloud SQL instance...`);
+    // If no schema has been deployed yet, deploy an empty one to get connectivity.
+    currentSchema = {
+      name: `${serviceName}/schemas/${SCHEMA_ID}`,
+      source: {
+        files: [],
+      },
+      primaryDatasource: {
+        postgresql: {
+          database: databaseId,
+          cloudSql: {
+            instance: instanceId,
+          },
+        },
+      },
+    };
   }
-  if (
-    !currentSchema.primaryDatasource.postgresql ||
-    currentSchema.primaryDatasource.postgresql.schemaValidation === "STRICT"
-  ) {
+  const postgresql = currentSchema.primaryDatasource.postgresql;
+  if (postgresql?.cloudSql.instance !== instanceId) {
+    logLabeledWarning(
+      "dataconnect",
+      `Switching connected Cloud SQL instance\nFrom ${postgresql?.cloudSql.instance}\nTo ${instanceId}`,
+    );
+  }
+  if (postgresql?.database !== databaseId) {
+    logLabeledWarning(
+      "dataconnect",
+      `Switching connected Postgres database from ${postgresql?.database} to ${databaseId}`,
+    );
+  }
+  if (!postgresql || postgresql.schemaValidation === "STRICT") {
     return;
   }
-  currentSchema.primaryDatasource.postgresql.schemaValidation = "STRICT";
+  postgresql.schemaValidation = "STRICT";
   try {
     await upsertSchema(currentSchema, /** validateOnly=*/ false);
   } catch (err: any) {