diff --git a/package.json b/package.json index c32f280f..61e26644 100644 --- a/package.json +++ b/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" }, diff --git a/release-notes.md b/release-notes.md index 3a1dea2b..3e8ddfbd 100644 --- a/release-notes.md +++ b/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 diff --git a/src/api/HalinContext.js b/src/api/HalinContext.js index ad505dc7..0171fd83 100644 --- a/src/api/HalinContext.js +++ b/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]; diff --git a/src/api/cluster/ClusterManager.js b/src/api/cluster/ClusterManager.js index b2b57798..db9aba45 100644 --- a/src/api/cluster/ClusterManager.js +++ b/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); diff --git a/src/api/cluster/ClusterMember.js b/src/api/cluster/ClusterMember.js index 73970969..5ee0a47d 100644 --- a/src/api/cluster/ClusterMember.js +++ b/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; }); } } diff --git a/src/api/data/DataFeed.js b/src/api/data/DataFeed.js index b54e8ec7..b6e406ce 100644 --- a/src/api/data/DataFeed.js +++ b/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; diff --git a/src/api/driver/errors.js b/src/api/driver/errors.js index cb2af3d5..cabe66e7 100644 --- a/src/api/driver/errors.js +++ b/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, }; - diff --git a/src/api/driver/errors.test.js b/src/api/driver/errors.test.js index ffa86636..4d46cf9a 100644 --- a/src/api/driver/errors.test.js +++ b/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); + }); + } }); }); \ No newline at end of file diff --git a/src/api/sentry/index.js b/src/api/sentry/index.js index e3ed0dfd..1e431de4 100644 --- a/src/api/sentry/index.js +++ b/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; }; diff --git a/src/components/database/ApocMetaStats/ApocMetaStats.js b/src/components/database/ApocMetaStats/ApocMetaStats.js index 9d1aa71e..b2677631 100644 --- a/src/components/database/ApocMetaStats/ApocMetaStats.js +++ b/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
+ Database is reconciling; please wait until it is fully online for stats to appear. +
; } - 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

{this.state.message}

; } - - return ( -
-

{this.state.nodeCount} nodes, {this.state.relCount} relationships, and -  {this.state.propertyKeyCount} properties.

- -

Labels

- - { - Object.keys(this.state.labels).length === 0 ? 'None' : - Object.keys(this.state.labels).map((label, i) => - {label}: {api.driver.handleNeo4jInt(this.state.labels[label])} nodes) - } - - -

Relationships

- - { - Object.keys(this.state.relTypes).length === 0 ? 'None' : - Object.keys(this.state.relTypes).map((rt, i) => - {rt}: {api.driver.handleNeo4jInt(this.state.relTypes[rt])}) - } - -
- ); - } + if (state.error) { return
{state.error}
; } + if (state.message) { return
{state.message}
; } + + return ( +
+

{state.nodeCount} nodes, {state.relCount} relationships, and +  {state.propertyKeyCount} properties.

+ +

Labels

+ + { + Object.keys(state.labels).length === 0 ? 'None' : + Object.keys(state.labels).map((label, i) => + {label}: {api.driver.handleNeo4jInt(state.labels[label])} nodes) + } + + +

Relationships

+ + { + Object.keys(state.relTypes).length === 0 ? 'None' : + Object.keys(state.relTypes).map((rt, i) => + {rt}: {api.driver.handleNeo4jInt(state.relTypes[rt])}) + } + +
+ ); } const Stats = hoc.apocOnlyComponent(ApocMetaStats); diff --git a/src/components/timeseries/ClusterTimeseries.js b/src/components/timeseries/ClusterTimeseries.js index b1ef3738..536843bf 100644 --- a/src/components/timeseries/ClusterTimeseries.js +++ b/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;