Work around WS in UI #3587
Work around WS in UI #3587
Conversation
| @@ -389,7 +389,7 @@ export default class Contract { | |||
| const subscriptions = Object.values(this._subscriptions) | |||
| .filter((s) => s.options.toBlock && s.options.toBlock === 'pending'); | |||
|
|
|||
| const timeout = () => setTimeout(() => this._subscribeFromPendings(), 1000); | |||
| const timeout = () => setTimeout(() => this._subscribeToPendings(), 1000); | |||
There was a problem hiding this comment.
Wasn't this fixed/merged? (Update with master?)
There was a problem hiding this comment.
Ah yes sorry, will update
| @@ -102,7 +102,7 @@ export default class Status { | |||
| }, timeout); | |||
| }; | |||
|
|
|||
| fetch('/', { method: 'HEAD' }) | |||
| fetch('/api/ping', { method: 'HEAD' }) | |||
There was a problem hiding this comment.
Anything under //api/* is not available on :8180, needs to be /. You cannot connect to :8080 either since to get the port you first need the API up via dappsUrl/dappsPort.
|
|
||
| console.log('SecureApi:constructor', sysuiToken); | ||
| // Try tokens from hash, then from localstorage | ||
| this._tokensToTry = [ nextToken, sysuiToken ].filter((t) => t && t.length); |
There was a problem hiding this comment.
Needs to be localStorage first, otherwise we end up with a bunch of single-use tokens. (Parity needs to clean this up, so we want to re-use as much as possible.)
| const url = process.env.PARITY_URL || window.location.host; | ||
|
|
||
| return fetch( | ||
| `http://${url}/api/ping`, |
There was a problem hiding this comment.
:8180 doesn't have /api/ping
|
|
||
| console.log('SecureApi:constructor', sysuiToken); | ||
| // Try tokens from hash, then from localstorage |
There was a problem hiding this comment.
Comment doesn't quite match the code anymore ;)
| } | ||
| } | ||
|
|
||
| _checkNodeUp () { |
There was a problem hiding this comment.
This doesn't really belong in the WS transport layer, the purpose it to manage the connection, not rely on how the rest is stitched together. Here we are now assuming that there is always a capable /api/ping available.
There was a problem hiding this comment.
Better now with the HEAD /, however we are assuming that the content is served from the same location as the WS now in the WS interface. Very specific...
| // Event code 1006 for WS means there is an error | ||
| // (not just closed by server) | ||
| if (up && event.code === 1006) { | ||
| event.status = 403; |
There was a problem hiding this comment.
Dangerous, but better than what we have atm. (Apart from the up check that doesn't belong)
There was a problem hiding this comment.
So the thing is that there is no way to know why a WS connection has been closed. It's like this in the specs. I will just remove this and put the checkNodeUp method in the secureApi
There was a problem hiding this comment.
Yeap. That is the issue - if we had a 403 like gets displayed in the console, it would be so much easier.
| '/api/*': { | ||
| proxy: [ | ||
| { | ||
| context: (pathname, req) => { |
There was a problem hiding this comment.
I learn something new every day.
|
Fixed comments |
|
|
||
| console.log('SecureApi:constructor', sysuiToken); | ||
| // Try tokens from localstorage, then from hash | ||
| this._tokensToTry = [ sysuiToken, nextToken ].filter((t) => t && t.length); |
There was a problem hiding this comment.
Ok, one last comment - as soon as we change anything here we need a full test of all the iterations -
- localStorage token cleared, none specified
- token set to initial in localStorage`
- localeStorage token, none specified
- invalid localStorage token
- 1, 2, 3 & 4 with
parity ui(calling /#/auth) - doing everything wrong and having to supply a token (both correct & incorrect)
- ...
e.g. also see we have autoConnect swapped in WS, could be fine with logic, but need to thoroughly re-test all scenarios we can think of here.
There was a problem hiding this comment.
I've already tested them all
|
Ok, last comment made on the re-testing (really critical, since I have burned once or twice on this by not testing each and every scenario). Apart from that, looks good. |
|
Changes Unknown when pulling 0c3d87f on ng-ws-improved into ** on master**. |
jacogr
added
A8-looksgood 🦄
|
Changes Unknown when pulling 0c3d87f on ng-ws-improved into ** on master**. |
This might help with #3544
This UI gets smarter about connection and the use of Signer Token.
There shouldn't be anymore the Enter Signer Token modal if a correct one is available.
Connection status should work better.
Close to no calls should be stacked up in the queue if Parity is disconnected.
In dev mode, one can do
window._parityWS._debug = trueto have some debug information about WS messages. It will print the number of requests made per second (on average) every 5 seconds, as well as the total number of calls made from the start of the application.Closes https://github.com/ethcore/parity/issues/3494