Skip to content
This repository has been archived by the owner on Dec 5, 2020. It is now read-only.

Replace EventEmitter-based commandClient #1

Open
jamesseanwright opened this issue Jul 17, 2016 · 1 comment
Open

Replace EventEmitter-based commandClient #1

jamesseanwright opened this issue Jul 17, 2016 · 1 comment

Comments

@jamesseanwright
Copy link
Owner

The current approach to abstracting the WebSocket connection, commandClient, is poorly designed. Due to its Pub/Sub nature, consuming it across the codebase results in potential race conditions.

Proposed replacement:

  • The new command client uses a Promise-based API, so consumers are more directly in control of invocation
  • Promises will return their own EventEmitter instances, so that Term4All can still interact with long-lived processes. This will also make subscription cleanup easier
@chilliyo
Copy link

chilliyo commented Jan 21, 2018

When I run this app the first time it works fine, but when I try to refresh the page, the back-end gives me the 'not opened' error.(see below) I'm guessing this issue you brought up is related to the error I am encountering? But I have a hard time understand why it is poorly designed and your proposed replacement. Could you help me to understand? I am new to web programming. Thanks in advance.

else throw new Error('not opened');
^

Error: not opened

------------------------Error in Console:-------------------------
commandClient.js:59 WebSocket connection to 'ws://18.xxx.xxx.xxx:9000/' failed: Error in connection establishment: net::ERR_CONNECTION_REFUSED
begin @ commandClient.js:59
(anonymous) @ commandsActions.js:47
(anonymous) @ index.js:11
runCommand @ AppContainer.js:24
ReactErrorUtils.invokeGuardedCallback @ ReactErrorUtils.js:70
executeDispatch @ EventPluginUtils.js:89
executeDispatchesInOrder @ EventPluginUtils.js:112
executeDispatchesAndRelease @ EventPluginHub.js:44
executeDispatchesAndReleaseTopLevel @ EventPluginHub.js:55
forEachAccumulated @ forEachAccumulated.js:24
processEventQueue @ EventPluginHub.js:221
runEventQueueInBatch @ ReactEventEmitterMixin.js:18
handleTopLevel @ ReactEventEmitterMixin.js:29
handleTopLevelImpl @ ReactEventListener.js:73
perform @ Transaction.js:138
batchedUpdates @ ReactDefaultBatchingStrategy.js:63
batchedUpdates @ ReactUpdates.js:99
dispatchEvent @ ReactEventListener.js:150

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants