feat: Try relaunching parity-ethereum node if it's not connecting after 15s#543
feat: Try relaunching parity-ethereum node if it's not connecting after 15s#543amaury1093 merged 11 commits intomasterfrom
Conversation
| break; | ||
| } | ||
| case 'RESTART_NODE_REQUEST': { | ||
| setupParityEthereum({}); |
There was a problem hiding this comment.
Well, if hasCalledInitParityEthereum was actually set to true where it should have been in the code, this would have thrown Unable to initialise Parity Ethereum more than once :P
There was a problem hiding this comment.
Also, setupParityEthereum takes a parameter and passes down .win to ParityEthereum, but the ParityEthereum constructor doesn't take any argument; let's fix this while we're at it.
There was a problem hiding this comment.
Well, if hasCalledInitParityEthereum was actually set to true where it should have been in the code, this would have thrown Unable to initialise Parity Ethereum more than once :p
We could for example:
- have the current constructor logic of ParityEthereum inside
(new ParityEthereum).init() - store ParityEthereum in FetherApp
- use fetherApp.parityEthereum.init() in the message handler
or not have ParityEthereum be a class...
There was a problem hiding this comment.
or not have ParityEthereum be a class...
lol, that's what I wanted to find somewhere in the code, but everything seems to be a class.
I just did as if setupParityEthereum was that function, didn't think too much about it. I propose just to remove the argument of this function, and not refactor too much (leave it for a next PR refactor PR). What do you think?
There was a problem hiding this comment.
The return value of setupParityEthereum (the instance of ParityEthereum) isn't used anywhere, and ParityEthereum is only used by setupParityEthereum. So I think it's safe to move the code of the constructor of ParityEthereum (minus hasCalledInitParityEthereum) as the contents of setupParityEthereum. I don't think it's too much work, but I don't mind if we do this in another PR
fixes #538
Repro:
Additional cleanup:
<RequireHealthOverlay require='node'>in the code (mostly in Accounts pages), and put one single<RequireHealthOverlay require='node'>in App.js. This is because App.js is wrapped in<RequireParityVersion>, which itself actually already requires<RequireHealthOverlay require='node'>(or else we have a blank screen, aka what ToB saw in their report)status.launching. At the beginning it was used to denote if parity-ethereum is in the process of launching, i.e. not api not connected yet, but process is launched. Not very useful.