From aba38721b18e30093e696fd3a37ae3dd846c5217 Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Wed, 21 Dec 2016 15:12:40 +0100 Subject: [PATCH] Fix Secure API hangs (#3927) * Use proxy for WS in dev * Update SecureAPI * Update webpack config * Fix dev contract * Update webpack * Linting fixes * Refactor Secure API logic : Promise based, no wastes of req * Fix tests * Add try 'intitial' token --- js/package.json | 3 +- js/src/api/transport/ws/ws.js | 61 +++++-- js/src/index.js | 7 +- js/src/redux/providers/status.js | 55 ------ js/src/redux/providers/statusReducer.js | 1 - js/src/secureApi.js | 163 ++++++++---------- js/src/serviceWorker.js | 2 +- js/src/ui/BlockStatus/blockStatus.js | 10 +- js/src/views/Connection/connection.js | 32 ++-- js/src/views/WriteContract/writeContract.css | 2 + js/src/views/WriteContract/writeContract.js | 11 +- .../views/WriteContract/writeContractStore.js | 4 +- js/webpack/app.js | 40 ++++- js/webpack/dev.server.js | 10 +- js/webpack/vendor.js | 2 + 15 files changed, 218 insertions(+), 185 deletions(-) diff --git a/js/package.json b/js/package.json index df59d43d9..b98fe163b 100644 --- a/js/package.json +++ b/js/package.json @@ -117,6 +117,7 @@ "react-hot-loader": "3.0.0-beta.6", "react-intl-aggregate-webpack-plugin": "0.0.1", "rucksack-css": "0.9.1", + "script-ext-html-webpack-plugin": "1.3.4", "serviceworker-webpack-plugin": "0.1.7", "sinon": "1.17.6", "sinon-as-promised": "4.0.2", @@ -125,7 +126,7 @@ "stylelint": "7.6.0", "stylelint-config-standard": "15.0.0", "url-loader": "0.5.7", - "webpack": "2.1.0-beta.27", + "webpack": "2.2.0-rc.1", "webpack-dev-middleware": "1.8.4", "webpack-error-notification": "0.1.6", "webpack-hot-middleware": "2.13.2", diff --git a/js/src/api/transport/ws/ws.js b/js/src/api/transport/ws/ws.js index 2de500309..43f1403ce 100644 --- a/js/src/api/transport/ws/ws.js +++ b/js/src/api/transport/ws/ws.js @@ -22,7 +22,7 @@ import TransportError from '../error'; /* global WebSocket */ export default class Ws extends JsonRpcBase { - constructor (url, token) { + constructor (url, token, connect = true) { super(); this._url = url; @@ -32,23 +32,34 @@ export default class Ws extends JsonRpcBase { this._connecting = false; this._connected = false; this._lastError = null; - this._autoConnect = true; + this._autoConnect = false; this._retries = 0; this._reconnectTimeoutId = null; - this._connect(); + this._connectPromise = null; + this._connectPromiseFunctions = {}; + + if (connect) { + this.connect(); + } } - updateToken (token) { + updateToken (token, connect = true) { this._token = token; - this._autoConnect = true; + // this._autoConnect = true; - this._connect(); + if (connect) { + this.connect(); + } } - _connect () { + connect () { + if (this._connected) { + return Promise.resolve(); + } + if (this._connecting) { - return; + return this._connectPromise || Promise.resolve(); } if (this._reconnectTimeoutId) { @@ -104,10 +115,17 @@ export default class Ws extends JsonRpcBase { window._parityWS = this; } + + this._connectPromise = new Promise((resolve, reject) => { + this._connectPromiseFunctions = { resolve, reject }; + }); + + return this._connectPromise; } _onOpen = (event) => { - console.log('ws:onOpen', event); + console.log('ws:onOpen'); + this._connected = true; this._connecting = false; this._autoConnect = true; @@ -116,6 +134,11 @@ export default class Ws extends JsonRpcBase { Object.keys(this._messages) .filter((id) => this._messages[id].queued) .forEach(this._send); + + this._connectPromiseFunctions.resolve(); + + this._connectPromise = null; + this._connectPromiseFunctions = {}; } _onClose = (event) => { @@ -135,13 +158,20 @@ export default class Ws extends JsonRpcBase { console.log('ws:onClose', `trying again in ${time}...`); this._reconnectTimeoutId = setTimeout(() => { - this._connect(); + this.connect(); }, timeout); return; } - console.log('ws:onClose', event); + if (this._connectPromise) { + this._connectPromiseFunctions.reject(event); + + this._connectPromise = null; + this._connectPromiseFunctions = {}; + } + + console.log('ws:onClose'); } _onError = (event) => { @@ -149,10 +179,17 @@ export default class Ws extends JsonRpcBase { // ie. don't print if error == closed window.setTimeout(() => { if (this._connected) { - console.error('ws:onError', event); + console.error('ws:onError'); event.timestamp = Date.now(); this._lastError = event; + + if (this._connectPromise) { + this._connectPromiseFunctions.reject(event); + + this._connectPromise = null; + this._connectPromiseFunctions = {}; + } } }, 50); } diff --git a/js/src/index.js b/js/src/index.js index b90f9c61a..77f247b3b 100644 --- a/js/src/index.js +++ b/js/src/index.js @@ -52,12 +52,7 @@ if (process.env.NODE_ENV === 'development') { } const AUTH_HASH = '#/auth?'; -const parityUrl = process.env.PARITY_URL || - ( - process.env.NODE_ENV === 'production' - ? window.location.host - : '127.0.0.1:8180' - ); +const parityUrl = process.env.PARITY_URL || window.location.host; let token = null; if (window.location.hash && window.location.hash.indexOf(AUTH_HASH) === 0) { diff --git a/js/src/redux/providers/status.js b/js/src/redux/providers/status.js index d7b360b14..ef4c09224 100644 --- a/js/src/redux/providers/status.js +++ b/js/src/redux/providers/status.js @@ -22,13 +22,11 @@ export default class Status { this._api = api; this._store = store; - this._pingable = false; this._apiStatus = {}; this._status = {}; this._longStatus = {}; this._minerSettings = {}; - this._pollPingTimeoutId = null; this._longStatusTimeoutId = null; this._timestamp = Date.now(); @@ -36,7 +34,6 @@ export default class Status { start () { this._subscribeBlockNumber(); - this._pollPing(); this._pollStatus(); this._pollLongStatus(); this._pollLogs(); @@ -65,50 +62,6 @@ export default class Status { }); } - /** - * Pinging should be smart. It should only - * be used when the UI is connecting or the - * Node is deconnected. - * - * @see src/views/Connection/connection.js - */ - _shouldPing = () => { - const { isConnected } = this._apiStatus; - return !isConnected; - } - - _stopPollPing = () => { - if (!this._pollPingTimeoutId) { - return; - } - - clearTimeout(this._pollPingTimeoutId); - this._pollPingTimeoutId = null; - } - - _pollPing = () => { - // Already pinging, don't try again - if (this._pollPingTimeoutId) { - return; - } - - const dispatch = (pingable, timeout = 1000) => { - if (pingable !== this._pingable) { - this._pingable = pingable; - this._store.dispatch(statusCollection({ isPingable: pingable })); - } - - this._pollPingTimeoutId = setTimeout(() => { - this._stopPollPing(); - this._pollPing(); - }, timeout); - }; - - fetch('/', { method: 'HEAD' }) - .then((response) => dispatch(!!response.ok)) - .catch(() => dispatch(false)); - } - _pollTraceMode = () => { return this._api.trace.block() .then(blockTraces => { @@ -137,7 +90,6 @@ export default class Status { if (gotConnected) { this._pollLongStatus(); - this._store.dispatch(statusCollection({ isPingable: true })); } if (!isEqual(apiStatus, this._apiStatus)) { @@ -145,13 +97,6 @@ export default class Status { this._apiStatus = apiStatus; } - // Ping if necessary, otherwise stop pinging - if (this._shouldPing()) { - this._pollPing(); - } else { - this._stopPollPing(); - } - if (!isConnected) { return nextTimeout(250); } diff --git a/js/src/redux/providers/statusReducer.js b/js/src/redux/providers/statusReducer.js index 413ff3319..17186b012 100644 --- a/js/src/redux/providers/statusReducer.js +++ b/js/src/redux/providers/statusReducer.js @@ -43,7 +43,6 @@ const initialState = { syncing: true, isConnected: false, isConnecting: false, - isPingable: false, isTest: undefined, refreshStatus: false, traceMode: undefined diff --git a/js/src/secureApi.js b/js/src/secureApi.js index d91a974d7..445151c69 100644 --- a/js/src/secureApi.js +++ b/js/src/secureApi.js @@ -14,35 +14,45 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . +import { uniq } from 'lodash'; + import Api from './api'; const sysuiToken = window.localStorage.getItem('sysuiToken'); export default class SecureApi extends Api { constructor (url, nextToken) { - super(new Api.Transport.Ws(url, sysuiToken)); + super(new Api.Transport.Ws(url, sysuiToken, false)); + this._url = url; this._isConnecting = true; - this._connectState = sysuiToken === 'initial' ? 1 : 0; this._needsToken = false; + this._dappsPort = 8080; this._dappsInterface = null; this._signerPort = 8180; - this._followConnectionTimeoutId = null; // Try tokens from localstorage, then from hash - this._tokensToTry = [ sysuiToken, nextToken ].filter((t) => t && t.length); + this._tokens = uniq([sysuiToken, nextToken, 'initial']) + .filter((token) => token) + .map((token) => ({ value: token, tried: false })); - this._followConnection(); + this._tryNextToken(); } - setToken = () => { + saveToken = () => { window.localStorage.setItem('sysuiToken', this._transport.token); - // DEBUG: console.log('SecureApi:setToken', this._transport.token); + // DEBUG: console.log('SecureApi:saveToken', this._transport.token); } + /** + * Returns a Promise that gets resolved with + * a boolean: `true` if the node is up, `false` + * otherwise + */ _checkNodeUp () { - return fetch('/', { method: 'HEAD' }) + const url = this._url.replace(/wss?/, 'http'); + return fetch(url, { method: 'HEAD' }) .then( (r) => r.status === 200, () => false @@ -50,92 +60,70 @@ export default class SecureApi extends Api { .catch(() => false); } - _followConnection = () => { - const nextTick = () => { - if (this._followConnectionTimeoutId) { - clearTimeout(this._followConnectionTimeoutId); - } + _setManual () { + this._needsToken = true; + this._isConnecting = false; + } - this._followConnectionTimeoutId = setTimeout(() => this._followConnection(), 250); - }; + _tryNextToken () { + const nextTokenIndex = this._tokens.findIndex((t) => !t.tried); - const setManual = () => { - this._connectState = 100; - this._needsToken = true; - this._isConnecting = false; - }; - - const lastError = this._transport.lastError; - const isConnected = this._transport.isConnected; - - switch (this._connectState) { - // token = - case 0: - if (isConnected) { - return this.connectSuccess(); - } else if (lastError) { - return this - ._checkNodeUp() - .then((isNodeUp) => { - const { timestamp } = lastError; - - if ((Date.now() - timestamp) > 250) { - return nextTick(); - } - - const nextToken = this._tokensToTry[0] || 'initial'; - const nextState = nextToken !== 'initial' ? 0 : 1; - - // If previous token was wrong (error while node up), delete it - if (isNodeUp) { - this._tokensToTry = this._tokensToTry.slice(1); - } - - if (nextToken !== this._transport.token) { - this.updateToken(nextToken, nextState); - } - - return nextTick(); - }); - } - break; - - // token = 'initial' - case 1: - if (isConnected) { - this.signer - .generateAuthorizationToken() - .then((token) => { - this.updateToken(token, 2); - }) - .catch((error) => { - console.error('SecureApi:generateAuthorizationToken', error); - setManual(); - }); - return; - } else if (lastError) { - return setManual(); - } - break; - - // token = - case 2: - if (isConnected) { - return this.connectSuccess(); - } else if (lastError) { - return setManual(); - } - break; + if (nextTokenIndex < 0) { + return this._setManual(); } - nextTick(); + const nextToken = this._tokens[nextTokenIndex]; + nextToken.tried = true; + + this.updateToken(nextToken.value); + } + + _followConnection = () => { + const token = this.transport.token; + + return this + .transport + .connect() + .then(() => { + if (token === 'initial') { + return this.signer + .generateAuthorizationToken() + .then((token) => { + return this.updateToken(token); + }) + .catch((e) => console.error(e)); + } + + this.connectSuccess(); + return true; + }) + .catch((e) => { + this + ._checkNodeUp() + .then((isNodeUp) => { + // Try again in a few... + if (!isNodeUp) { + this._isConnecting = false; + const timeout = this.transport.retryTimeout; + + window.setTimeout(() => { + this._followConnection(); + }, timeout); + + return; + } + + this._tryNextToken(); + return false; + }); + }); } connectSuccess () { this._isConnecting = false; this._needsToken = false; - this.setToken(); + this.saveToken(); Promise .all([ @@ -152,10 +140,9 @@ export default class SecureApi extends Api { // DEBUG: console.log('SecureApi:connectSuccess', this._transport.token); } - updateToken (token, connectState = 0) { - this._connectState = connectState; - this._transport.updateToken(token.replace(/[^a-zA-Z0-9]/g, '')); - this._followConnection(); + updateToken (token) { + this._transport.updateToken(token.replace(/[^a-zA-Z0-9]/g, ''), false); + return this._followConnection(); // DEBUG: console.log('SecureApi:updateToken', this._transport.token, connectState); } diff --git a/js/src/serviceWorker.js b/js/src/serviceWorker.js index c558a57cf..136e6a6b7 100644 --- a/js/src/serviceWorker.js +++ b/js/src/serviceWorker.js @@ -140,5 +140,5 @@ function getCompiler (build) { }); } - return self.solc[longVersion]; + return Promise.resolve(self.solc[longVersion]); } diff --git a/js/src/ui/BlockStatus/blockStatus.js b/js/src/ui/BlockStatus/blockStatus.js index df6f44a8c..f50c7a685 100644 --- a/js/src/ui/BlockStatus/blockStatus.js +++ b/js/src/ui/BlockStatus/blockStatus.js @@ -62,9 +62,17 @@ class BlockStatus extends Component { ); } + let syncStatus = null; + + if (syncing && syncing.currentBlock && syncing.highestBlock) { + syncStatus = ( + { syncing.currentBlock.toFormat() }/{ syncing.highestBlock.toFormat() } syncing + ); + } + return (
- { syncing.currentBlock.toFormat() }/{ syncing.highestBlock.toFormat() } syncing + { syncStatus } { warpStatus }
); diff --git a/js/src/views/Connection/connection.js b/js/src/views/Connection/connection.js index 8cc2378d9..51b0c8018 100644 --- a/js/src/views/Connection/connection.js +++ b/js/src/views/Connection/connection.js @@ -34,27 +34,26 @@ class Connection extends Component { static propTypes = { isConnected: PropTypes.bool, isConnecting: PropTypes.bool, - isPingable: PropTypes.bool, needsToken: PropTypes.bool } state = { + loading: false, token: '', validToken: false } render () { - const { isConnected, isConnecting, isPingable } = this.props; - const isOk = !isConnecting && isConnected && isPingable; + const { isConnected, needsToken } = this.props; - if (isOk) { + if (isConnected) { return null; } - const typeIcon = isPingable + const typeIcon = needsToken ? : ; - const description = isPingable + const description = needsToken ? this.renderSigner() : this.renderPing(); @@ -82,7 +81,7 @@ class Connection extends Component { } renderSigner () { - const { token, validToken } = this.state; + const { loading, token, validToken } = this.state; const { isConnecting, needsToken } = this.props; if (needsToken && !isConnecting) { @@ -93,9 +92,11 @@ class Connection extends Component { + onChange={ this.onChangeToken } + /> ); @@ -117,7 +118,7 @@ class Connection extends Component { } onChangeToken = (event, token) => { - const validToken = /[a-zA-Z0-9]{4}-[a-zA-Z0-9]{4}-[a-zA-Z0-9]{4}-[a-zA-Z0-9]{4}/.test(token); + const validToken = /[a-zA-Z0-9]{4}-?[a-zA-Z0-9]{4}-?[a-zA-Z0-9]{4}-?[a-zA-Z0-9]{4}/.test(token); this.setState({ token, validToken }, () => { validToken && this.setToken(); }); @@ -127,15 +128,20 @@ class Connection extends Component { const { api } = this.context; const { token } = this.state; - api.updateToken(token, 0); - this.setState({ token: '', validToken: false }); + this.setState({ loading: true }); + + api + .updateToken(token, 0) + .then((isValid) => { + this.setState({ loading: isValid || false, validToken: isValid }); + }); } } function mapStateToProps (state) { - const { isConnected, isConnecting, isPingable, needsToken } = state.nodeStatus; + const { isConnected, isConnecting, needsToken } = state.nodeStatus; - return { isConnected, isConnecting, isPingable, needsToken }; + return { isConnected, isConnecting, needsToken }; } function mapDispatchToProps (dispatch) { diff --git a/js/src/views/WriteContract/writeContract.css b/js/src/views/WriteContract/writeContract.css index c5cefcf7a..3db18bf39 100644 --- a/js/src/views/WriteContract/writeContract.css +++ b/js/src/views/WriteContract/writeContract.css @@ -105,6 +105,8 @@ display: flex; flex-direction: column; margin-right: 0.5em; + overflow: auto; + .panel { background-color: rgba(0, 0, 0, 0.5); padding: 1em; diff --git a/js/src/views/WriteContract/writeContract.js b/js/src/views/WriteContract/writeContract.js index c95c09c04..8adb80b5a 100644 --- a/js/src/views/WriteContract/writeContract.js +++ b/js/src/views/WriteContract/writeContract.js @@ -21,6 +21,7 @@ import { connect } from 'react-redux'; import { bindActionCreators } from 'redux'; import CircularProgress from 'material-ui/CircularProgress'; import moment from 'moment'; +import { throttle } from 'lodash'; import ContentClear from 'material-ui/svg-icons/content/clear'; import SaveIcon from 'material-ui/svg-icons/content/save'; @@ -60,6 +61,8 @@ class WriteContract extends Component { if (worker !== undefined) { this.store.setWorker(worker); } + + this.throttledResize = throttle(this.applyResize, 100, { leading: true }); } componentDidMount () { @@ -516,10 +519,16 @@ class WriteContract extends Component { const x = pageX - left; - this.setState({ size: 100 * x / width }); + this.size = 100 * x / width; + this.throttledResize(); + event.stopPropagation(); } + applyResize = () => { + this.setState({ size: this.size }); + } + } function mapStateToProps (state) { diff --git a/js/src/views/WriteContract/writeContractStore.js b/js/src/views/WriteContract/writeContractStore.js index 141569af2..a1f710f8e 100644 --- a/js/src/views/WriteContract/writeContractStore.js +++ b/js/src/views/WriteContract/writeContractStore.js @@ -288,7 +288,7 @@ export default class WriteContractStore { const build = this.builds[this.selectedBuild]; const version = build.longVersion; - const sourcecode = this.sourcecode.replace(/\n+/g, '\n').replace(/\s(\s+)/g, ' '); + const sourcecode = this.sourcecode.replace(/\s+/g, ' '); const hash = sha3(JSON.stringify({ version, sourcecode, optimize: this.optimize })); let promise = Promise.resolve(null); @@ -302,7 +302,7 @@ export default class WriteContractStore { } else { promise = this .compile({ - sourcecode: sourcecode, + sourcecode: this.sourcecode, build: build, optimize: this.optimize, files: this.files diff --git a/js/webpack/app.js b/js/webpack/app.js index df801533c..9044d470b 100644 --- a/js/webpack/app.js +++ b/js/webpack/app.js @@ -23,6 +23,7 @@ const CopyWebpackPlugin = require('copy-webpack-plugin'); const HtmlWebpackPlugin = require('html-webpack-plugin'); const ExtractTextPlugin = require('extract-text-webpack-plugin'); const ServiceWorkerWebpackPlugin = require('serviceworker-webpack-plugin'); +const ScriptExtHtmlWebpackPlugin = require('script-ext-html-webpack-plugin'); const Shared = require('./shared'); const DAPPS = require('../src/dapps'); @@ -35,10 +36,13 @@ const isProd = ENV === 'production'; module.exports = { cache: !isProd, - devtool: isProd ? '#eval' : '#eval-source-map', + devtool: isProd ? '#hidden-source-map' : '#source-map', context: path.join(__dirname, '../src'), entry: Object.assign({}, Shared.dappsEntry, { + modals: './modals/index.js', + views: './views/index.js', + ui: './ui/index.js', index: './index.js' }), output: { @@ -162,13 +166,43 @@ module.exports = { filename: 'index.html', template: './index.ejs', favicon: FAVICON, - chunks: [ isProd ? null : 'commons', 'index' ] + chunks: [ + isProd ? null : 'commons', + 'common.modals', 'common.views', 'common.ui', + 'modals', 'views', 'ui', + 'index' + ] + }), + + new ScriptExtHtmlWebpackPlugin({ + sync: [ 'commons', 'vendor.js' ], + defaultAttribute: 'defer' }), new ServiceWorkerWebpackPlugin({ entry: path.join(__dirname, '../src/serviceWorker.js') }), + new webpack.optimize.CommonsChunkPlugin({ + filename: 'commons.modals.[hash:10].js', + name: 'common.modals', + minChunks: 2, + chunks: [ 'index', 'modals' ] + }), + + new webpack.optimize.CommonsChunkPlugin({ + filename: 'commons.views.[hash:10].js', + name: 'common.views', + minChunks: 2, + chunks: [ 'index', 'views' ] + }), + + new webpack.optimize.CommonsChunkPlugin({ + filename: 'commons.ui.[hash:10].js', + name: 'common.ui', + minChunks: 2 + }), + DappsHTMLInjection ); @@ -185,7 +219,7 @@ module.exports = { new webpack.optimize.CommonsChunkPlugin({ filename: 'commons.[hash:10].js', name: 'commons', - minChunks: Infinity + minChunks: 2 }) ); } diff --git a/js/webpack/dev.server.js b/js/webpack/dev.server.js index fc107488a..ccfd2026f 100644 --- a/js/webpack/dev.server.js +++ b/js/webpack/dev.server.js @@ -22,6 +22,7 @@ const webpackHotMiddleware = require('webpack-hot-middleware'); const http = require('http'); const express = require('express'); const ProgressBar = require('progress'); +const proxy = require('http-proxy-middleware'); const webpackConfig = require('./app'); const Shared = require('./shared'); @@ -33,6 +34,8 @@ let progressBar = { update: () => {} }; * and HMR to the plugins */ (function updateWebpackConfig () { + webpackConfig.performance = { hints: false }; + Object.keys(webpackConfig.entry).forEach((key) => { const entry = webpackConfig.entry[key]; @@ -81,13 +84,18 @@ app.use(webpackDevMiddleware(compiler, { } })); -app.use(express.static(webpackConfig.output.path)); +var wsProxy = proxy('ws://127.0.0.1:8180', { changeOrigin: true }); // Add the dev proxies in the express App Shared.addProxies(app); +app.use(express.static(webpackConfig.output.path)); +app.use(wsProxy); + const server = http.createServer(app); server.listen(process.env.PORT || 3000, function () { console.log('Listening on port', server.address().port); progressBar = new ProgressBar('[:bar] :percent :etas', { total: 50 }); }); + +server.on('upgrade', wsProxy.upgrade); diff --git a/js/webpack/vendor.js b/js/webpack/vendor.js index 7f1114144..daacab379 100644 --- a/js/webpack/vendor.js +++ b/js/webpack/vendor.js @@ -31,9 +31,11 @@ let modules = [ 'ethereumjs-tx', 'lodash', 'material-ui', + 'material-ui-chip-input', 'mobx', 'mobx-react', 'moment', + 'phoneformat.js', 'react', 'react-dom', 'react-redux',