From ce34112c4e1af44d494c2d5b0eb04ffecc9fe3c1 Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Wed, 23 Oct 2019 16:58:51 +0200 Subject: [PATCH 1/4] console.warn when parsing error --- .../src/utils/PostMessageProvider.js | 31 ++++++++++++++++--- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/packages/fether-react/src/utils/PostMessageProvider.js b/packages/fether-react/src/utils/PostMessageProvider.js index a984c0b1e..4bcaf5f2c 100644 --- a/packages/fether-react/src/utils/PostMessageProvider.js +++ b/packages/fether-react/src/utils/PostMessageProvider.js @@ -18,7 +18,7 @@ import * as postMessage from './postMessage'; import EventEmitter from 'eventemitter3'; export default class PostMessageProvider extends EventEmitter { - constructor (destination, source) { + constructor (destination) { super(); this._destination = destination || window.parent; @@ -136,7 +136,17 @@ export default class PostMessageProvider extends EventEmitter { } _receiveMessage (raw) { - const parsed = JSON.parse(raw); + // FIXME I'm not sure how to fix this + // I had some occasions where it cannot parse + let parsed; + try { + parsed = JSON.parse(raw); + } catch (err) { + console.warn(`Cannot parse ${raw}. Ignoring message.`); + + return; + } + const { id, error } = parsed; const subscription = parsed.params && parsed.params.subscription; @@ -144,12 +154,25 @@ export default class PostMessageProvider extends EventEmitter { // subscription notification const result = parsed.params.result; let messageId = this._subscriptionsToId[subscription]; - this._messages[messageId].callback(error && new Error(error), result); + + // FIXME I'm not sure how to fix this + // Sometimes we receive results for a subscription that we have never + // seen before. Ignore. + if (!this._messages[messageId]) { + console.warn(`Got result for unknown subscription ${subscription}`); + + return; + } + + this._messages[messageId].callback( + error && new Error(error.message), + result + ); } else { // request response const result = parsed.result; if (error) { - this._messages[id].reject(new Error(error)); + this._messages[id].reject(new Error(error.message)); } else { this._messages[id].resolve(result); From e6c45a59a128dbaa65aed149404426bacf94ffca Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Wed, 23 Oct 2019 17:26:51 +0200 Subject: [PATCH 2/4] Add some logs --- .../fether-electron/src/main/app/ipcChannel/index.js | 2 ++ packages/fether-react/src/utils/PostMessageProvider.js | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/packages/fether-electron/src/main/app/ipcChannel/index.js b/packages/fether-electron/src/main/app/ipcChannel/index.js index 90d8b01c0..2efaba688 100644 --- a/packages/fether-electron/src/main/app/ipcChannel/index.js +++ b/packages/fether-electron/src/main/app/ipcChannel/index.js @@ -33,6 +33,8 @@ class IpcChannel extends EventEmitter { }); socket.on('data', data_ => { const data = data_.toString(); + + console.log('GOT IPC DATA', data); // Sometimes we receive multiple messages at once const messages = data.split(/\r?\n/).filter(Boolean); messages.forEach(data => { diff --git a/packages/fether-react/src/utils/PostMessageProvider.js b/packages/fether-react/src/utils/PostMessageProvider.js index 4bcaf5f2c..864fe7fee 100644 --- a/packages/fether-react/src/utils/PostMessageProvider.js +++ b/packages/fether-react/src/utils/PostMessageProvider.js @@ -169,6 +169,15 @@ export default class PostMessageProvider extends EventEmitter { result ); } else { + // FIXME I'm not sure how to fix this + // Sometimes we receive results for an id that we have never seen before. + // Ignore. + if (!this._messages[id]) { + console.warn(`Got result for unknown id ${id}`); + + return; + } + // request response const result = parsed.result; if (error) { From e49be11302faedcec8c93607f975a73a7ce83f0e Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Thu, 24 Oct 2019 11:51:35 +0200 Subject: [PATCH 3/4] Concatenate previous message if ill-formed --- .../src/main/app/ipcChannel/index.js | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/packages/fether-electron/src/main/app/ipcChannel/index.js b/packages/fether-electron/src/main/app/ipcChannel/index.js index 2efaba688..0f25e9ec7 100644 --- a/packages/fether-electron/src/main/app/ipcChannel/index.js +++ b/packages/fether-electron/src/main/app/ipcChannel/index.js @@ -31,12 +31,25 @@ class IpcChannel extends EventEmitter { this._sendQueued(); resolve(true); }); + + // We get data from the socket by chunks, and 1 chunk !== 1 Rpc message + // Sometimes 1 chunk is multiple messages, sometimes we get partial + // messages. + // https://github.com/paritytech/fether/issues/562 + + // The last element in a '\n'-separate chunk. Will be `""` if the + // last element was a correctly-formed message, or will hold the + // ill-formed message otherwise. + let lastData = ''; socket.on('data', data_ => { - const data = data_.toString(); + // If the last data was truncated, then we concatenate to the new data + // we just received + const data = `${lastData}${data_.toString()}`; + + // All messages are separated by a '\n' + const messages = data.split(/\n/); + lastData = messages.pop(); // Will hold "" or an ill-formed JSONRPC message - console.log('GOT IPC DATA', data); - // Sometimes we receive multiple messages at once - const messages = data.split(/\r?\n/).filter(Boolean); messages.forEach(data => { this.emit('message', data); }); From 7339101b5355ac773b049011d4d118ebcb96946c Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Thu, 24 Oct 2019 11:56:19 +0200 Subject: [PATCH 4/4] Use debug instead of console.log --- .../src/utils/PostMessageProvider.js | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/packages/fether-react/src/utils/PostMessageProvider.js b/packages/fether-react/src/utils/PostMessageProvider.js index 864fe7fee..f0f03f356 100644 --- a/packages/fether-react/src/utils/PostMessageProvider.js +++ b/packages/fether-react/src/utils/PostMessageProvider.js @@ -14,9 +14,13 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . -import * as postMessage from './postMessage'; import EventEmitter from 'eventemitter3'; +import Debug from './debug'; +import * as postMessage from './postMessage'; + +const debug = Debug('PostMessageProvider'); + export default class PostMessageProvider extends EventEmitter { constructor (destination) { super(); @@ -136,13 +140,13 @@ export default class PostMessageProvider extends EventEmitter { } _receiveMessage (raw) { - // FIXME I'm not sure how to fix this - // I had some occasions where it cannot parse let parsed; try { parsed = JSON.parse(raw); } catch (err) { - console.warn(`Cannot parse ${raw}. Ignoring message.`); + // Should not happen anymore, since the following issue is fixed + // https://github.com/paritytech/fether/issues/562 + debug(`Cannot parse ${raw}. Ignoring message.`); return; } @@ -155,11 +159,10 @@ export default class PostMessageProvider extends EventEmitter { const result = parsed.params.result; let messageId = this._subscriptionsToId[subscription]; - // FIXME I'm not sure how to fix this // Sometimes we receive results for a subscription that we have never - // seen before. Ignore. + // seen before. Should not happen. if (!this._messages[messageId]) { - console.warn(`Got result for unknown subscription ${subscription}`); + debug(`Got result for unknown subscription ${subscription}`); return; } @@ -169,11 +172,10 @@ export default class PostMessageProvider extends EventEmitter { result ); } else { - // FIXME I'm not sure how to fix this // Sometimes we receive results for an id that we have never seen before. - // Ignore. + // Should not happen. if (!this._messages[id]) { - console.warn(`Got result for unknown id ${id}`); + debug(`Got result for unknown id ${id}`); return; }