V0.14.1 (#142)

* error reporting improvements

* close neo4j 4.0 sessions

* improved error testing

* stop/remove datafeeds when database is dropped

* fix race condition when feed is stopping

* stop and restart data feeds when database stops and starts

* simplify component using useEffect

package.json

@@ -1,7 +1,7 @@
 {
   "name": "halin",
   "description": "Halin helps you monitor and improve your Neo4j graph",
-  "version": "0.14.0",
+  "version": "0.14.1",
   "neo4jDesktop": {
     "apiVersion": "^1.2.0"
   },

release-notes.md

@@ -1,5 +1,11 @@
 # Halin Release Notes
 
+## 0.14.1 Bugfix Release
+
+- Better error handling for common Neo4j errors
+- DB-specific data feeds get stopped & started along with the underlying database stop/restart.
+- Fixed some data feed errors caused by underlying race conditions in data return
+
 ## 0.14.0 Transactions & GC
 
 - Revamped garbage collection to be a scatterplot of instantaneous GC

src/api/HalinContext.js

@@ -116,10 +116,40 @@ export default class HalinContext {
         return this.mgr;
     }
 
+    /**
+     * Get a set of DataFeeds that apply to a given database
+     * @param {String} db database name
+     * @return {Array} of DataFeed objects.
+     */
+    getFeedsForDatabase(db) {
+        return _.values(this.dataFeeds).filter(df => df.database === db);
+    }
+
+    /**
+     * Get a set of DataFeeds that apply to a cluster member.
+     * @param {ClusterMember} clusterMember 
+     * @return {Array} of DataFeed objects.
+     */
     getFeedsFor(clusterMember) {
         return _.values(this.dataFeeds).filter(df => df.node === clusterMember);
     }
 
+    /**
+     * Remove a DataFeed from the halin context.  This shuts the data feed down.
+     * @param {DataFeed} df 
+     * @returns true if the feed was in this halin context, false otherwise.
+     */
+    removeDataFeed(df) {
+        df.stop();
+
+        if (this.dataFeeds[df.name]) {
+            delete this.dataFeeds[df.name];
+            return true;
+        }
+
+        return false;
+    }
+
     getDataFeed(feedOptions) {
         const df = new DataFeed(feedOptions);
         const feed = this.dataFeeds[df.name];

src/api/cluster/ClusterManager.js

@@ -454,6 +454,12 @@ export default class ClusterManager extends Metric {
     stopDatabase(db) {
         if (!db || !db.name) { throw new Error('Invalid or missing database'); }
 
+        const feeds = this.ctx.getFeedsForDatabase(db.name);
+        feeds.forEach(df => {
+            sentry.fine(`Stopping data feed for ${db.name}`);
+            df.stop();
+        });
+
         return this.ctx.getSystemDBWriter().run(`STOP DATABASE \`${db.name}\``, {}, neo4j.SYSTEM_DB)
             .then(results => {
                 sentry.info('stop results', results);
@@ -482,12 +488,27 @@ export default class ClusterManager extends Metric {
                 alert: true,
                 message: `Started database ${db.name}`,
                 payload: db,
-            }));
+            }))
+            .then(() => {
+                // If any data feeds existed for this DB, restart them.  This can occur
+                // when user has feeds, stops database, and then restarts them.  The feed
+                // is still valid, but is paused while the DB is inactive.
+                const feeds = this.ctx.getFeedsForDatabase(db.name);
+                feeds.forEach(df => {
+                    sentry.fine(`Starting data feed for ${db.name}`);
+                    df.start();
+                });                
+            })
     }
 
     dropDatabase(db) {
         if (!db || !db.name) { throw new Error('Invalid or missing database'); }
 
+        const dataFeeds = this.ctx.getFeedsForDatabase(db.name);
+        dataFeeds.forEach(df => {
+            sentry.info(`Stopping data feed on ${db.name}`, this.ctx.removeDataFeed(df));
+        });
+
         return this.ctx.getSystemDBWriter().run(`DROP DATABASE \`${db.name}\``, {}, neo4j.SYSTEM_DB)
             .then(results => {
                 sentry.info('drop results', results);

src/api/cluster/ClusterMember.js

@@ -617,8 +617,13 @@ export default class ClusterMember {
             })
             // Cleanup session.
             .finally(p => {
-                return poolSession ? this.pool.release(s)
-                    .catch(e => sentry.fine('Pool release error', e)) : p;
+                if (poolSession) {
+                    this.pool.release(s).catch(e => sentry.fine('Pool release error', e));
+                } else {
+                    s.close();
+                }
+
+                return p;
             });
     }
 }

src/api/data/DataFeed.js

@@ -126,7 +126,6 @@ export default class DataFeed extends Metric {
         const existingNames = this.aliases.map(nameAliasSet);
 
         if (existingNames.indexOf(thisName) === -1) {
-            console.log('novel aliases pushed');
             this.aliases.push(aliases);
         }
         return this.aliases;

src/api/driver/errors.js

@@ -1,21 +1,48 @@
 const asStr = err => `${err}`;
 
 const matches = (substr) => (err) => asStr(err).toLowerCase().indexOf(substr.toLowerCase()) > -1;
+const contains = (err, str) => asStr(err).toLowerCase().indexOf(str.toLowerCase()) > -1;
 
+const permissionDenied = matches('permission denied');
+const noProcedure = matches('no procedure with the name');
+const unauthorized = matches('unauthorized');
+const bookmarks = matches('Supplied bookmark');
+const failedToEstablishConnection = matches('failed to establish connection in');
+const browserSecurityConstraints = matches('security constraints in your web browser');
+const noActiveDatabase = matches('active database');
+const insecureWSFromHTTPS = matches('insecure websocket connection may not be initiated from a page loaded over HTTPS');
+const repeatedAuthFailure = matches('incorrect authentication details too many times in a row');
+const fileNotFound = matches('java.io.FileNotFoundException');
+const connectionRefused = matches('ERR_CONNECTION_REFUSED');
+const apocFileImportNotEnabled = matches('apoc.import.file.enabled');
+const notUpToRequestedVersion = matches('database not up to requested version');
+
+const isNeo4jError = err =>
+    failedToEstablishConnection(err) || browserSecurityConstraints(err) ||
+    permissionDenied(err) || noProcedure(err) ||
+    unauthorized(err) || bookmarks(err) ||
+    noActiveDatabase(err) || insecureWSFromHTTPS(err) ||
+    repeatedAuthFailure(err) || fileNotFound(err) ||
+    connectionRefused(err) || notUpToRequestedVersion(err);
+
+const isAPOCError = err => apocFileImportNotEnabled(err);
+
 export default {
+    isNeo4jError,
+    isAPOCError,
     matches,
-    permissionDenied: matches('permission denied'),
-    noProcedure: matches('no procedure with the name'),
-    unauthorized: matches('unauthorized'),
-    failedToEstablishConnection: matches('failed to establish connection in'),
-    browserSecurityConstraints: matches('security constraints in your web browser'),
-    noActiveDatabase: matches('active database'),
-    contains: (err, str) => asStr(err).toLowerCase().indexOf(str.toLowerCase()) > -1,
-    insecureWSFromHTTPS: matches('insecure websocket connection may not be initiated from a page loaded over HTTPS'),
-    repeatedAuthFailure: matches('incorrect authentication details too many times in a row'),
-    fileNotFound: matches('java.io.FileNotFoundException'),
-    connectionRefused: matches('ERR_CONNECTION_REFUSED'),
-    apocFileImportNotEnabled: matches('apoc.import.file.enabled'),
-    notUpToRequestedVersion: matches('database not up to requested version'),
+    permissionDenied,
+    noProcedure,
+    unauthorized,
+    bookmarks,
+    failedToEstablishConnection,
+    browserSecurityConstraints,
+    noActiveDatabase,
+    contains,
+    insecureWSFromHTTPS,
+    repeatedAuthFailure,
+    fileNotFound,
+    connectionRefused,
+    apocFileImportNotEnabled,
+    notUpToRequestedVersion,
 };
-

src/api/driver/errors.test.js

@@ -33,5 +33,17 @@ describe('Neo4j Driver Error Detection', function () {
             expect(fn(sampleError)).toBe(true);
             expect(fn(bogusError)).toBe(false);
         });
+
+        if (key.indexOf('apoc') < 0) {
+            it(`Knows that error ${key} is a Neo4j Error`, () => {
+                const sampleError = new Error(errTypes[key]);
+                expect(errors.isNeo4jError(sampleError)).toBe(true);
+            });
+        } else {
+            it(`Knows that ${key} is an APOC error`, () => {
+                const sampleError = new Error(errTypes[key]);
+                expect(errors.isAPOCError(sampleError)).toBe(true);
+            });
+        }
     });
 });

src/api/sentry/index.js

@@ -6,6 +6,7 @@
 import * as Sentry from '@sentry/browser';
 import appPkg from '../../package.json';
 import _ from 'lodash';
+import errors from '../driver/errors';
 
 let initialized = false;
 let enabled = true;
@@ -34,6 +35,8 @@ const shouldSentryCapture = err => {
     const href = _.get(window, 'location.href');
     if (href && href.indexOf('localhost') > -1) {
         return false;
+    } else if(errors.isNeo4jError(err)) {
+        return false;
     }
 
     return true;
@@ -60,9 +63,11 @@ const reportError = (err, message=null) => {
         Sentry.captureException(err);
         if (message) {
             console.error(message, err);
-        }    
+        } 
+        return err;
     }
 
+    console.log('Sentry skipped reporting error');
     return err;
 };
 

src/components/database/ApocMetaStats/ApocMetaStats.js

@@ -1,6 +1,5 @@
-import React, { Component } from 'react';
+import React, { Component, useState, useEffect } from 'react';
 import PropTypes from 'prop-types';
-
 import api from '../../../api';
 
 import HalinCard from '../../ui/scaffold/HalinCard/HalinCard';
@@ -16,7 +15,6 @@ import './ApocMetaStats.css';
  */
 const gatherStats = (node, database) => {
     if (!node || !database) { return null; }
-
     if (database.isReconciling()) {
         // Database has multiple statuses, and it's not safe to do this operation at this moment.
         return Promise.resolve({ reconciling: true });
@@ -61,60 +59,59 @@ const gatherStats = (node, database) => {
         });
 };
 
-class ApocMetaStats extends Component {
-    state = {
-        relCount: 0,
-        nodeCount: 0,
-        labelCount: 0,
-        propertyKeyCount: 0,
-        labels: {},
-        relTypes: {},
-        message: null,
-    };
-
-    UNSAFE_componentWillReceiveProps(props) {
-        return gatherStats(props.node, props.database)
-            .then(state => this.setState(state));
-    }
+const defaultState = {
+    relCount: 0,
+    nodeCount: 0,
+    labelCount: 0,
+    propertyKeyCount: 0,
+    labels: {},
+    relTypes: {},
+    message: null,
+};
 
-    componentDidMount() {
-        return gatherStats(this.props.node, this.props.database)
-            .then(state => this.setState(state));
+const ApocMetaStats = (props) => {
+    const [state, setStats] = useState(defaultState);
+    useEffect(() => {
+       async function getStats() {
+           const state = await gatherStats(props.node, props.database);
+           setStats(state);
+       }
+       getStats();
+    }, [props.database, props.node]);
+
+    if (state.reconciling) {
+        return <div>
+            Database is reconciling; please wait until it is fully online for stats to appear.
+            </div>;
     }
 
-    render() {
-        if (this.state.reconciling) {
-            return `Database is reconciling; please wait until it is fully online for stats to appear.`;
-        }
-
-        if (this.state.error) { return `${this.state.error}`; }
-        if (this.state.message) { return <p>{this.state.message}</p>; }
-
-        return (
-            <div>
-                <p>{this.state.nodeCount} nodes, {this.state.relCount} relationships, and
-                &nbsp;{this.state.propertyKeyCount} properties.</p>
-
-                <h4>Labels</h4>
-                <List id='label_list'>
-                    {
-                        Object.keys(this.state.labels).length === 0 ? 'None' :
-                            Object.keys(this.state.labels).map((label, i) =>
-                                <List.Item key={i}>{label}: {api.driver.handleNeo4jInt(this.state.labels[label])} nodes</List.Item>)
-                    }
-                </List>
-
-                <h4>Relationships</h4>
-                <List id='rel_list'>
-                    {
-                        Object.keys(this.state.relTypes).length === 0 ? 'None' :
-                            Object.keys(this.state.relTypes).map((rt, i) =>
-                                <List.Item key={i}>{rt}: {api.driver.handleNeo4jInt(this.state.relTypes[rt])}</List.Item>)
-                    }
-                </List>
-            </div>
-        );
-    }
+    if (state.error) { return <div>{state.error}</div>; }
+    if (state.message) { return <div>{state.message}</div>; }
+
+    return (
+        <div>
+            <p>{state.nodeCount} nodes, {state.relCount} relationships, and
+            &nbsp;{state.propertyKeyCount} properties.</p>
+
+            <h4>Labels</h4>
+            <List id='label_list'>
+                {
+                    Object.keys(state.labels).length === 0 ? 'None' :
+                        Object.keys(state.labels).map((label, i) =>
+                            <List.Item key={i}>{label}: {api.driver.handleNeo4jInt(state.labels[label])} nodes</List.Item>)
+                }
+            </List>
+
+            <h4>Relationships</h4>
+            <List id='rel_list'>
+                {
+                    Object.keys(state.relTypes).length === 0 ? 'None' :
+                        Object.keys(state.relTypes).map((rt, i) =>
+                            <List.Item key={i}>{rt}: {api.driver.handleNeo4jInt(state.relTypes[rt])}</List.Item>)
+                }
+            </List>
+        </div>
+    );
 }
 
 const Stats = hoc.apocOnlyComponent(ApocMetaStats);

src/components/timeseries/ClusterTimeseries.js

@@ -207,7 +207,7 @@ class ClusterTimeseries extends Component {
                 // We're doing a property switch, and we're replacing a previous data feed.
                 // This requires that we unregister the previous hooks we had in place, so
                 // that the component doesn't get confused with double updates.
-                console.log('removing old data listener');
+                // console.log('removing old data listener');
                 this.feeds[addr].removeListener('data', this.onDataCallbacks[addr]);
             }
 
@@ -278,6 +278,12 @@ class ClusterTimeseries extends Component {
         );*/
 
         const futurePad = 1000; // ms into the future to show blank space on graph
+
+        if (!dataFeed.feedStartTime) {
+            sentry.fine('ClusterTimeseries:  feed is stopped');
+            return null;
+        }
+
         const fst = dataFeed.feedStartTime.getTime();
 
         let startTime, endTime;