ui: fix query fetching store ids for db/tables

These queries were updated in cockroachdb#118904 to make them more performant.
In that change we mistakenly removed the null check before reading
the `rows` field, causing pages using this request to crash if this
query fails.

Epic: none
Fixes: cockroachdb#126348

Release note (bug fix): Database overview and db details
pages should not crash if the range information is not
available.

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

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

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

@@ -179,6 +179,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);
 }
 
@@ -197,6 +198,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

@@ -464,12 +464,16 @@ const getTableReplicaStoreIDs: TableDetailsQuery<TableReplicasRow> = {
   ) => {
     if (txnResult.error) {
       resp.stats.replicaData.error = txnResult.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 = txnResult?.rows[0]?.store_ids ?? [];
-    resp.stats.replicaData.replicaCount =
-      txnResult?.rows[0]?.replica_count ?? 0;
+    if (!txnResultIsEmpty(txnResult)) {
+      resp.stats.replicaData.storeIDs = txnResult?.rows[0]?.store_ids ?? [];
+      resp.stats.replicaData.replicaCount =
+        txnResult?.rows[0]?.replica_count ?? 0;
+    }
   },
 };
 

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

@@ -19,7 +19,7 @@ import {
   REMOTE_DEBUGGING_ERROR_TEXT,
   indexUnusedDuration,
 } from "src/util/constants";
-import { stubSqlApiCall } from "src/util/fakeApi";
+import { mockExecSQLErrors, stubSqlApiCall } from "src/util/fakeApi";
 
 import * as api from "./api";
 import fetchMock from "./fetch-mock";
@@ -304,6 +304,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 () {
@@ -498,6 +512,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

@@ -11,6 +11,7 @@
 import * as $protobuf from "protobufjs";
 import { api as clusterUiApi } from "@cockroachlabs/cluster-ui";
 import moment from "moment-timezone";
+import { SqlTxnResult } from "@cockroachlabs/cluster-ui/dist/types/api";
 
 import { cockroach } from "src/js/protos";
 import { API_PREFIX, STATUS_PREFIX } from "src/util/api";
@@ -106,7 +107,17 @@ export function stubSqlApiCall<T>(
   mockTxnResults: mockSqlTxnResult<T>[],
   times = 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 }),
+  );
+};