Skip to content

Commit

Permalink
cluster-ui: handle partial response errors on the databases page
Browse files Browse the repository at this point in the history 
Part of: #102386

Prior to this change, any query error in the reponse payload for the
databases page would cause the page to render an error. This was
problematic as some queries used to power the databases page directly
query `system` tables, meaning only `ADMIN` users could access the page.
This change allows the databases page to handle network responses with
query errors, consequently allowing non-admin users to view the data
they are privy to.

On the databases page, two types of requests are made:
- a single request to fetch all database names
- a request to fetch the database details for each database name

The error handling for these requests has changed as such:
- if we encounter a network or a non size-related query error when
  requesting database names, render a page error

- if we encounter a 'max size' query error when requesting database
  names, render an alert that we're showing partial results

- if we encounter any error requesting a database's details, render a
  `Caution` icon next to the database's name to indicate there was an
issue getting results, the `Caution` icon has a tooltip providing a
general explanation as to what the issue is

- network errors when fetching database details provide no data for the
  database's table row, consequently the row of statistics for that
database is `unavailable`, the network error message is provided in the
`Caution` icon tooltip

- query errors when fetching database details are scoped to the row
  cells for that query, which are `unavailable`

- `unavailable` cells have a tooltip that highlight the error for that
  cell as well

Release note (ui change): Allow non-admin users to view the databases
page.
  • Loading branch information
Thomas Hardy committed Aug 28, 2023
1 parent ee2b0cc commit d77bee0
Show file tree
Hide file tree
Showing 12 changed files with 333 additions and 170 deletions.
2 changes: 1 addition & 1 deletion pkg/sql/planner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -962,7 +962,7 @@ func (p *planner) GetDetailsForSpanStats(
return p.QueryIteratorEx(
ctx,
"crdb_internal.database_span_stats",
sessiondata.NoSessionDataOverride,
sessiondata.NodeUserSessionDataOverride,
query,
args...,
)
Expand Down
63 changes: 40 additions & 23 deletions pkg/ui/workspaces/cluster-ui/src/api/databaseDetailsApi.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
// licenses/APL.txt.

import {
combineQueryErrors,
createSqlExecutionRequest,
executeInternalSql,
formatApiResult,
Expand All @@ -17,6 +18,7 @@ import {
SqlApiResponse,
SqlExecutionErrorMessage,
SqlExecutionRequest,
sqlResultsAreEmpty,
SqlStatement,
SqlTxnResult,
txnResultIsEmpty,
Expand Down Expand Up @@ -100,7 +102,7 @@ const getDatabaseId: DatabaseDetailsQuery<DatabaseIdRow> = {
}
},
handleMaxSizeError: (_dbName, _response, _dbDetail) => {
return new Promise<boolean>(() => false);
return Promise.resolve(false);
},
};

Expand Down Expand Up @@ -138,12 +140,12 @@ const getDatabaseGrantsQuery: DatabaseDetailsQuery<DatabaseGrantsRow> = {
}
},
handleMaxSizeError: (_dbName, _response, _dbDetail) => {
return new Promise<boolean>(() => false);
return Promise.resolve(false);
},
};

// Database Tables
type DatabaseTablesResponse = {
export type DatabaseTablesResponse = {
tables: string[];
};

Expand Down Expand Up @@ -281,11 +283,11 @@ const getDatabaseZoneConfig: DatabaseDetailsQuery<DatabaseZoneConfigRow> = {
}
}
if (txn_result.error) {
resp.idResp.error = txn_result.error;
resp.zoneConfigResp.error = txn_result.error;
}
},
handleMaxSizeError: (_dbName, _response, _dbDetail) => {
return new Promise<boolean>(() => false);
return Promise.resolve(false);
},
};

Expand All @@ -296,7 +298,7 @@ type DatabaseDetailsStats = {
indexStats: SqlApiQueryResponse<DatabaseIndexUsageStatsResponse>;
};

type DatabaseSpanStatsRow = {
export type DatabaseSpanStatsRow = {
approximate_disk_bytes: number;
live_bytes: number;
total_bytes: number;
Expand Down Expand Up @@ -338,7 +340,7 @@ const getDatabaseSpanStats: DatabaseDetailsQuery<DatabaseSpanStatsRow> = {
}
},
handleMaxSizeError: (_dbName, _response, _dbDetail) => {
return new Promise<boolean>(() => false);
return Promise.resolve(false);
},
};

Expand Down Expand Up @@ -383,7 +385,7 @@ const getDatabaseReplicasAndRegions: DatabaseDetailsQuery<DatabaseReplicasRegion
}
},
handleMaxSizeError: (_dbName, _response, _dbDetail) => {
return new Promise<boolean>(() => false);
return Promise.resolve(false);
},
};

Expand Down Expand Up @@ -428,7 +430,7 @@ const getDatabaseIndexUsageStats: DatabaseDetailsQuery<IndexUsageStatistic> = {
}
},
handleMaxSizeError: (_dbName, _response, _dbDetail) => {
return new Promise<boolean>(() => false);
return Promise.resolve(false);
},
};

Expand Down Expand Up @@ -467,12 +469,15 @@ const databaseDetailQueries: DatabaseDetailsQuery<DatabaseDetailsRow>[] = [
export function createDatabaseDetailsReq(
params: DatabaseDetailsReqParams,
): SqlExecutionRequest {
return createSqlExecutionRequest(
params.database,
databaseDetailQueries.map(query =>
query.createStmt(params.database, params.csIndexUnusedDuration),
return {
...createSqlExecutionRequest(
params.database,
databaseDetailQueries.map(query =>
query.createStmt(params.database, params.csIndexUnusedDuration),
),
),
);
separate_txns: true,
};
}

export async function getDatabaseDetails(
Expand All @@ -488,30 +493,36 @@ async function fetchDatabaseDetails(
const detailsResponse: DatabaseDetailsResponse = newDatabaseDetailsResponse();
const req: SqlExecutionRequest = createDatabaseDetailsReq(params);
const resp = await executeInternalSql<DatabaseDetailsRow>(req);
const errs: Error[] = [];
resp.execution.txn_results.forEach(txn_result => {
if (txn_result.rows) {
const query: DatabaseDetailsQuery<DatabaseDetailsRow> =
databaseDetailQueries[txn_result.statement - 1];
query.addToDatabaseDetail(txn_result, detailsResponse);
if (txn_result.error) {
errs.push(txn_result.error);
}
const query: DatabaseDetailsQuery<DatabaseDetailsRow> =
databaseDetailQueries[txn_result.statement - 1];
query.addToDatabaseDetail(txn_result, detailsResponse);
});
if (resp.error) {
if (resp.error.message.includes("max result size exceeded")) {
if (isMaxSizeError(resp.error.message)) {
return fetchSeparatelyDatabaseDetails(params);
}
detailsResponse.error = resp.error;
}

detailsResponse.error = combineQueryErrors(errs, detailsResponse.error);
return formatApiResult<DatabaseDetailsResponse>(
detailsResponse,
detailsResponse.error,
"retrieving database details information",
`retrieving database details information for database '${params.database}'`,
false,
);
}

async function fetchSeparatelyDatabaseDetails(
params: DatabaseDetailsReqParams,
): Promise<SqlApiResponse<DatabaseDetailsResponse>> {
const detailsResponse: DatabaseDetailsResponse = newDatabaseDetailsResponse();
const errs: Error[] = [];
for (const databaseDetailQuery of databaseDetailQueries) {
const req = createSqlExecutionRequest(params.database, [
databaseDetailQuery.createStmt(
Expand All @@ -520,10 +531,14 @@ async function fetchSeparatelyDatabaseDetails(
),
]);
const resp = await executeInternalSql<DatabaseDetailsRow>(req);
if (sqlResultsAreEmpty(resp)) {
continue;
}
const txn_result = resp.execution.txn_results[0];
if (txn_result.rows) {
databaseDetailQuery.addToDatabaseDetail(txn_result, detailsResponse);
if (txn_result.error) {
errs.push(txn_result.error);
}
databaseDetailQuery.addToDatabaseDetail(txn_result, detailsResponse);

if (resp.error) {
const handleFailure = await databaseDetailQuery.handleMaxSizeError(
Expand All @@ -537,9 +552,11 @@ async function fetchSeparatelyDatabaseDetails(
}
}

detailsResponse.error = combineQueryErrors(errs, detailsResponse.error);
return formatApiResult<DatabaseDetailsResponse>(
detailsResponse,
detailsResponse.error,
"retrieving database details information",
`retrieving database details information for database '${params.database}'`,
false,
);
}
35 changes: 15 additions & 20 deletions pkg/ui/workspaces/cluster-ui/src/api/databasesApi.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,30 +42,25 @@ export const databasesRequest: SqlExecutionRequest = {
// getDatabasesList from cluster-ui will need to pass a timeout argument for
// promise timeout handling (callers from db-console already have promise
// timeout handling as part of the cacheDataReducer).
export function getDatabasesList(
export async function getDatabasesList(
timeout?: moment.Duration,
): Promise<DatabasesListResponse> {
return withTimeout(
const resp = await withTimeout(
executeInternalSql<DatabasesColumns>(databasesRequest),
timeout,
).then(result => {
// If there is an error and there are no result throw error.
const noTxnResultsExist = result?.execution?.txn_results?.length === 0;
if (
result.error &&
(noTxnResultsExist || result.execution.txn_results[0].rows.length === 0)
) {
throw result.error;
}
);
// Encountered a response level error, or empty response.
if (resp.error || sqlResultsAreEmpty(resp)) {
return { databases: [], error: resp.error };
}

if (sqlResultsAreEmpty(result)) {
return { databases: [], error: result.error };
}
// Get database names.
const dbNames: string[] = resp.execution.txn_results[0].rows.map(
row => row.database_name,
);

const dbNames: string[] = result.execution.txn_results[0].rows.map(
row => row.database_name,
);

return { databases: dbNames, error: result.error };
});
// Note that we do not surface the txn_result error in the returned payload.
// This request only contains a single txn_result, any error encountered by the txn_result
// will be surfaced at the response level by the sql api.
return { databases: dbNames, error: resp.error };
}
53 changes: 47 additions & 6 deletions pkg/ui/workspaces/cluster-ui/src/api/sqlApi.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
// licenses/APL.txt.

import { fetchDataJSON } from "./fetchData";
import { getLogger } from "../util";

export type SqlExecutionRequest = {
statements: SqlStatement[];
Expand All @@ -17,6 +18,7 @@ export type SqlExecutionRequest = {
application_name?: string; // Defaults to '$ api-v2-sql'
database?: string; // Defaults to system
max_result_size?: number; // Default 10kib
separate_txns?: boolean;
};

export type SqlStatement = {
Expand Down Expand Up @@ -165,7 +167,8 @@ export function sqlApiErrorMessage(message: string): string {

message = message.replace("run-query-via-api: ", "");
if (message.includes(":")) {
return message.split(":")[1];
const idx = message.indexOf(":") + 1;
return idx < message.length ? message.substring(idx) : message;
}

return message;
Expand All @@ -184,23 +187,43 @@ export function createSqlExecutionRequest(
};
}

export function isSeparateTxnError(message: string): boolean {
return !!message?.includes(
"separate transaction payload encountered transaction error",
);
}

export function isMaxSizeError(message: string): boolean {
return !!message?.includes("max result size exceeded");
}

export function isPrivilegeError(code: string): boolean {
return code === "42501";
}

export function formatApiResult<ResultType>(
results: ResultType,
error: SqlExecutionErrorMessage,
errorMessageContext: string,
shouldThrowOnQueryError = true,
): SqlApiResponse<ResultType> {
const maxSizeError = isMaxSizeError(error?.message);

if (error && !maxSizeError) {
throw new Error(
`Error while ${errorMessageContext}: ${sqlApiErrorMessage(
error?.message,
)}`,
);
if (shouldThrowOnQueryError) {
throw new Error(
`Error while ${errorMessageContext}: ${sqlApiErrorMessage(
error?.message,
)}`,
);
} else {
// Otherwise, just log.
getLogger().warn(
`Error while ${errorMessageContext}: ${sqlApiErrorMessage(
error?.message,
)}`,
);
}
}

return {
Expand All @@ -209,6 +232,24 @@ export function formatApiResult<ResultType>(
};
}

export function combineQueryErrors(
errs: Error[],
sqlError?: SqlExecutionErrorMessage,
): SqlExecutionErrorMessage {
if (errs.length === 0 && !sqlError) {
return;
}
const errMsgs = errs.map(err => `\n-` + sqlApiErrorMessage(err.message));
let sqlErrMsg = sqlError.message;
if (isSeparateTxnError(sqlErrMsg)) {
sqlErrMsg = "Encountered query error(s) fetching data:";
}
return {
...sqlError,
message: [sqlErrMsg, ...errMsgs].join(``),
};
}

export function txnResultIsEmpty(txn_result: SqlTxnResult<unknown>): boolean {
return !txn_result.rows || txn_result.rows?.length === 0;
}
17 changes: 4 additions & 13 deletions pkg/ui/workspaces/cluster-ui/src/databases/combiners.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import { DatabasesListResponse, SqlExecutionErrorMessage } from "../api";
import { DatabasesPageDataDatabase } from "../databasesPage";
import {
buildIndexStatToRecommendationsMap,
combineLoadingErrors,
getNodesByRegionString,
normalizePrivileges,
normalizeRoles,
Expand Down Expand Up @@ -70,8 +69,6 @@ const deriveDatabaseDetails = (
isTenant: boolean,
): DatabasesPageDataDatabase => {
const dbStats = dbDetails?.data?.results.stats;
const sizeInBytes = dbStats?.spanStats?.approximate_disk_bytes || 0;
const rangeCount = dbStats?.spanStats.range_count || 0;
const nodes = dbStats?.replicaData.replicas || [];
const nodesByRegionString = getNodesByRegionString(
nodes,
Expand All @@ -81,20 +78,14 @@ const deriveDatabaseDetails = (
const numIndexRecommendations =
dbStats?.indexStats.num_index_recommendations || 0;

const combinedErr = combineLoadingErrors(
dbDetails?.lastError,
dbDetails?.data?.maxSizeReached,
dbListError?.message,
);

return {
loading: !!dbDetails?.inFlight,
loaded: !!dbDetails?.valid,
lastError: combinedErr,
requestError: dbDetails?.lastError,
queryError: dbDetails?.data?.results?.error,
name: database,
sizeInBytes: sizeInBytes,
tableCount: dbDetails?.data?.results.tablesResp.tables?.length || 0,
rangeCount: rangeCount,
spanStats: dbStats?.spanStats,
tables: dbDetails?.data?.results.tablesResp,
nodes: nodes,
nodesByRegionString,
numIndexRecommendations,
Expand Down
Loading

0 comments on commit d77bee0

Please sign in to comment.