Merge pull request #127090 from xinhaoz/backport23.1-126419

release-23.1: ui: fix query fetching store ids for db/tables

pkg/ui/workspaces/cluster-ui/src/api/databaseDetailsApi.ts

@@ -370,8 +370,12 @@ const getDatabaseReplicasAndRegions: DatabaseDetailsQuery<DatabaseReplicasRegion
     ) => {
       if (txn_result.error) {
         resp.stats.replicaData.error = txn_result.error;
+        // We don't expect to have any rows for this query on error.
+        return;
+      }
+      if (!txnResultIsEmpty(txn_result)) {
+        resp.stats.replicaData.storeIDs = txn_result?.rows[0]?.store_ids ?? [];
       }
-      resp.stats.replicaData.storeIDs = txn_result?.rows[0]?.store_ids ?? [];
     },
     handleMaxSizeError: (_dbName, _response, _dbDetail) => {
       return Promise.resolve(false);

pkg/ui/workspaces/cluster-ui/src/api/sqlApi.ts

@@ -177,6 +177,7 @@ const UPGRADE_RELATED_ERRORS = [
 ];
 
 export function isUpgradeError(message: string): boolean {
+  if (message == null) return false;
   return UPGRADE_RELATED_ERRORS.some(err => message.search(err) !== -1);
 }
 
@@ -195,6 +196,10 @@ export function isUpgradeError(message: string): boolean {
  * @param message
  */
 export function sqlApiErrorMessage(message: string): string {
+  if (!message) {
+    return "";
+  }
+
   if (isUpgradeError(message)) {
     return "This page may not be available during an upgrade.";
   }

pkg/ui/workspaces/cluster-ui/src/api/tableDetailsApi.ts

@@ -463,12 +463,16 @@ const getTableReplicaStoreIDs: TableDetailsQuery<TableReplicasRow> = {
   ) => {
     if (txn_result.error) {
       resp.stats.replicaData.error = txn_result.error;
+      // We don't expect to have any rows for this query on error.
+      return;
     }
 
     // TODO #118957 (xinhaoz) Store IDs and node IDs cannot be used interchangeably.
-    resp.stats.replicaData.storeIDs = txn_result?.rows[0]?.store_ids ?? [];
-    resp.stats.replicaData.replicaCount =
-      txn_result?.rows[0]?.replica_count ?? 0;
+    if (!txnResultIsEmpty(txn_result)) {
+      resp.stats.replicaData.storeIDs = txn_result?.rows[0]?.store_ids ?? [];
+      resp.stats.replicaData.replicaCount =
+        txn_result?.rows[0]?.replica_count ?? 0;
+    }
   },
 };
 

pkg/ui/workspaces/db-console/src/util/api.spec.ts

@@ -23,7 +23,7 @@ import {
   indexUnusedDuration,
 } from "src/util/constants";
 import Severity = protos.cockroach.util.log.Severity;
-import { stubSqlApiCall } from "src/util/fakeApi";
+import { mockExecSQLErrors, stubSqlApiCall } from "src/util/fakeApi";
 
 const { ZoneConfig } = cockroach.config.zonepb;
 const { ZoneConfigurationLevel } = cockroach.server.serverpb;
@@ -285,6 +285,20 @@ describe("rest api", function () {
           done();
         });
     });
+
+    it("should not error when any query fails", async () => {
+      const req = { database, csIndexUnusedDuration: indexUnusedDuration };
+      const mockResults = mockExecSQLErrors<clusterUiApi.DatabaseDetailsRow>(6);
+      stubSqlApiCall<clusterUiApi.DatabaseDetailsRow>(
+        clusterUiApi.createDatabaseDetailsReq(req),
+        mockResults,
+      );
+
+      // This call should not throw.
+      const result = await clusterUiApi.getDatabaseDetails(req);
+      expect(result.results).toBeDefined();
+      expect(result.results.error).toBeDefined();
+    });
   });
 
   describe("table details request", function () {
@@ -479,6 +493,27 @@ describe("rest api", function () {
           done();
         });
     });
+
+    it("should not error when any query fails", async () => {
+      const mockResults = mockExecSQLErrors<clusterUiApi.TableDetailsRow>(10);
+      stubSqlApiCall<clusterUiApi.TableDetailsRow>(
+        clusterUiApi.createTableDetailsReq(
+          dbName,
+          tableName,
+          indexUnusedDuration,
+        ),
+        mockResults,
+      );
+
+      // This call should not throw.
+      const result = await clusterUiApi.getTableDetails({
+        database: dbName,
+        table: tableName,
+        csIndexUnusedDuration: indexUnusedDuration,
+      });
+      expect(result.results).toBeDefined();
+      expect(result.results.error).toBeDefined();
+    });
   });
 
   describe("events request", function () {

pkg/ui/workspaces/db-console/src/util/fakeApi.ts

@@ -9,6 +9,7 @@
 // licenses/APL.txt.
 
 import * as $protobuf from "protobufjs";
+import { SqlTxnResult } from "@cockroachlabs/cluster-ui/dist/types/api";
 
 import { api as clusterUiApi } from "@cockroachlabs/cluster-ui";
 import { cockroach } from "src/js/protos";
@@ -106,7 +107,17 @@ export function stubSqlApiCall<T>(
   mockTxnResults: mockSqlTxnResult<T>[],
   times: number = 1,
 ) {
-  const response = buildSqlExecutionResponse(mockTxnResults);
+  const firstError = mockTxnResults.find(mock => mock.error != null)?.error;
+  let err: clusterUiApi.SqlExecutionErrorMessage;
+  if (firstError != null) {
+    err = {
+      message: firstError.message,
+      code: "123",
+      severity: "ERROR",
+      source: { file: "myfile.go", line: 111, function: "myFn" },
+    };
+  }
+  const response = buildSqlExecutionResponse(mockTxnResults, err);
   fetchMock.mock({
     headers: {
       Accept: "application/json",
@@ -185,3 +196,23 @@ function buildSqlTxnResult<RowType>(
     error: mock.error,
   };
 }
+
+const mockStmtExecErrorResponse = <T>(
+  res: Partial<SqlTxnResult<T>>,
+): SqlTxnResult<T> => ({
+  statement: res?.statement ?? 1,
+  tag: "SELECT",
+  start: "2022-01-01T00:00:00Z",
+  end: "2022-01-01T00:00:01Z",
+  error: new Error("error"),
+  rows_affected: 0,
+  ...res,
+});
+
+export const mockExecSQLErrors = <T>(
+  statements: number,
+): mockSqlTxnResult<T>[] => {
+  return Array.from({ length: statements }, (_, i) =>
+    mockStmtExecErrorResponse<T>({ statement: i + 1 }),
+  );
+};