ui: render errors in Loading component

Previously, data fetching errors were not being consistently surfaced to the user. This commit updates all existing uses of the Loading component to surface these errors: - Some parent components render data from multiple sources, potentially having multiple errors to show the user. This commit also extends the Loading component to handle multiple errors. - The Certificates page had custom loading logic. This commit replaces logic with Loading component. Also in this commit is some minor cleanup (copy, layout). Fixes: cockroachdb#24011 Release note (admin ui change): This commit wires up data errors to all existing uses of the Loading component. Previously, data errors weren't consistently surfaced.
celiala · Nov 21, 2018 · 89687a8 · 89687a8
1 parent b503956
commit 89687a8
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 9 deletions.
diff --git a/pkg/ui/ccl/src/views/clusterviz/containers/map/nodeCanvasContainer.tsx b/pkg/ui/ccl/src/views/clusterviz/containers/map/nodeCanvasContainer.tsx
@@ -41,6 +41,7 @@ interface NodeCanvasContainerProps {
   livenesses: { [id: string]: Liveness };
   dataExists: boolean;
   dataIsValid: boolean;
+  dataErrors: Error[];
   refreshNodes: typeof refreshNodes;
   refreshLiveness: typeof refreshLiveness;
   refreshLocations: typeof refreshLocations;
@@ -72,6 +73,7 @@ class NodeCanvasContainer extends React.Component<NodeCanvasContainerProps & Nod
     return (
       <Loading
         loading={!this.props.dataExists}
+        error={this.props.dataErrors}
         render={() => (
           <NodeCanvas
             localityTree={currentLocality}
@@ -100,6 +102,13 @@ const selectDataIsValid = createSelector(
   (nodes, locations, liveness) => nodes.valid && locations.valid && liveness.valid,
 );
 
+const dataErrors = createSelector(
+  selectNodeRequestStatus,
+  selectLocationsRequestStatus,
+  selectLivenessRequestStatus,
+  (nodes, locations, liveness) => [nodes.lastError, locations.lastError, liveness.lastError],
+);
+
 export default connect(
   (state: AdminUIState, _ownProps: NodeCanvasContainerOwnProps) => ({
     nodesSummary: nodesSummarySelector(state),
@@ -109,6 +118,7 @@ export default connect(
     livenesses: livenessByNodeIDSelector(state),
     dataIsValid: selectDataIsValid(state),
     dataExists: selectDataExists(state),
+    dataErrors: dataErrors(state),
   }),
   {
     refreshNodes,

diff --git a/pkg/ui/src/views/cluster/containers/dataDistribution/index.tsx b/pkg/ui/src/views/cluster/containers/dataDistribution/index.tsx
@@ -24,11 +24,17 @@ import * as docsURL from "src/util/docs";
 import { FixLong } from "src/util/fixLong";
 import { cockroach } from "src/js/protos";
 import { AdminUIState } from "src/redux/state";
-import { refreshDataDistribution, refreshNodes, refreshLiveness } from "src/redux/apiReducers";
+import {
+  refreshDataDistribution,
+  refreshNodes,
+  refreshLiveness,
+  CachedDataReducerState,
+} from "src/redux/apiReducers";
 import { LocalityTree, selectLocalityTree } from "src/redux/localities";
 import ReplicaMatrix, { SchemaObject } from "./replicaMatrix";
 import { TreeNode, TreePath } from "./tree";
 import "./index.styl";
+import {selectLivenessRequestStatus, selectNodeRequestStatus} from "src/redux/nodes";
 
 type DataDistributionResponse = cockroach.server.serverpb.DataDistributionResponse;
 type NodeDescriptor = cockroach.roachpb.INodeDescriptor;
@@ -43,7 +49,7 @@ const ZONE_CONFIG_TEXT = (
 );
 
 interface DataDistributionProps {
-  dataDistribution: DataDistributionResponse;
+  dataDistribution: CachedDataReducerState<DataDistributionResponse>;
   localityTree: LocalityTree;
   sortedZoneConfigs: ZoneConfig$Properties[];
 }
@@ -69,7 +75,7 @@ class DataDistribution extends React.Component<DataDistributionProps> {
   getCellValue = (dbPath: TreePath, nodePath: TreePath): number => {
     const [dbName, tableName] = dbPath;
     const nodeID = nodePath[nodePath.length - 1];
-    const databaseInfo = this.props.dataDistribution.database_info;
+    const databaseInfo = this.props.dataDistribution.data.database_info;
 
     const res = databaseInfo[dbName].table_info[tableName].replica_count_by_node_id[nodeID];
     if (!res) {
@@ -81,7 +87,7 @@ class DataDistribution extends React.Component<DataDistributionProps> {
   render() {
     const nodeTree = nodeTreeFromLocalityTree("Cluster", this.props.localityTree);
 
-    const databaseInfo = this.props.dataDistribution.database_info;
+    const databaseInfo = this.props.dataDistribution.data.database_info;
     const dbTree: TreeNode<SchemaObject> = {
       name: "Cluster",
       data: {
@@ -137,8 +143,9 @@ class DataDistribution extends React.Component<DataDistributionProps> {
 }
 
 interface DataDistributionPageProps {
-  dataDistribution: DataDistributionResponse;
+  dataDistribution: CachedDataReducerState<DataDistributionResponse>;
   localityTree: LocalityTree;
+  localityTreeErrors: Error[];
   sortedZoneConfigs: ZoneConfig$Properties[];
   refreshDataDistribution: typeof refreshDataDistribution;
   refreshNodes: typeof refreshNodes;
@@ -170,8 +177,8 @@ class DataDistributionPage extends React.Component<DataDistributionPageProps> {
         </section>
         <section className="section">
           <Loading
-            className="loading-image loading-image__spinner-left"
-            loading={!this.props.dataDistribution || !this.props.localityTree}
+            loading={!this.props.dataDistribution.data || !this.props.localityTree}
+            error={[this.props.dataDistribution.lastError, ...this.props.localityTreeErrors]}
             render={() => (
               <DataDistribution
                 localityTree={this.props.localityTree}
@@ -196,12 +203,19 @@ const sortedZoneConfigs = createSelector(
   },
 );
 
+const localityTreeErrors = createSelector(
+  selectNodeRequestStatus,
+  selectLivenessRequestStatus,
+  (nodes, liveness) => [nodes.lastError, liveness.lastError],
+);
+
 // tslint:disable-next-line:variable-name
 const DataDistributionPageConnected = connect(
   (state: AdminUIState) => ({
-    dataDistribution: state.cachedData.dataDistribution.data,
+    dataDistribution: state.cachedData.dataDistribution,
     sortedZoneConfigs: sortedZoneConfigs(state),
     localityTree: selectLocalityTree(state),
+    localityTreeErrors: localityTreeErrors(state),
   }),
   {
     refreshDataDistribution,

diff --git a/pkg/ui/src/views/reports/containers/localities/index.tsx b/pkg/ui/src/views/reports/containers/localities/index.tsx
@@ -111,6 +111,7 @@ class Localities extends React.Component<LocalitiesProps, {}> {
         <section className="section"><h1>Localities</h1></section>
         <Loading
           loading={ !this.props.localityStatus.data || !this.props.locationStatus.data }
+          error={ [this.props.localityStatus.lastError, this.props.locationStatus.lastError] }
           render={() => (
             <section className="section">
               <table className="locality-table">

diff --git a/pkg/ui/src/views/reports/containers/network/index.tsx b/pkg/ui/src/views/reports/containers/network/index.tsx
@@ -20,9 +20,16 @@ import React from "react";
 import { Helmet } from "react-helmet";
 import { connect } from "react-redux";
 import { RouterState } from "react-router";
+import { createSelector } from "reselect";
 
 import { refreshLiveness, refreshNodes } from "src/redux/apiReducers";
-import { LivenessStatus, NodesSummary, nodesSummarySelector } from "src/redux/nodes";
+import {
+  LivenessStatus,
+  NodesSummary,
+  nodesSummarySelector,
+  selectLivenessRequestStatus,
+  selectNodeRequestStatus,
+} from "src/redux/nodes";
 import { AdminUIState } from "src/redux/state";
 import { LongToMoment, NanoToMilli } from "src/util/convert";
 import { FixLong } from "src/util/fixLong";
@@ -36,6 +43,7 @@ import Loading from "src/views/shared/components/loading";
 
 interface NetworkOwnProps {
   nodesSummary: NodesSummary;
+  nodeSummaryErrors: Error[];
   refreshNodes: typeof refreshNodes;
   refreshLiveness: typeof refreshLiveness;
 }
@@ -456,6 +464,7 @@ class Network extends React.Component<NetworkProps, {}> {
         <h1>Network Diagnostics</h1>
         <Loading
           loading={!contentAvailable(nodesSummary)}
+          error={this.props.nodeSummaryErrors}
           className="loading-image loading-image__spinner-left loading-image__spinner-left__padded"
           render={() => (
             <div>
@@ -469,9 +478,16 @@ class Network extends React.Component<NetworkProps, {}> {
   }
 }
 
+const nodeSummaryErrors = createSelector(
+  selectNodeRequestStatus,
+  selectLivenessRequestStatus,
+  (nodes, liveness) => [nodes.lastError, liveness.lastError],
+);
+
 function mapStateToProps(state: AdminUIState) {
   return {
     nodesSummary: nodesSummarySelector(state),
+    nodeSummaryErrors: nodeSummaryErrors(state),
   };
 }