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

fix offline-startup edge-case #1571

Merged
merged 3 commits into from
Jan 18, 2017
Merged

fix offline-startup edge-case #1571

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
002affc
fix offline-startup edge-case
Jan 10, 2017
0d6d2d7
Merge branch 'develop' into luclu_fix-offline-startup-edge-case
frozeman Jan 18, 2017
c5c8a89
Merge branch 'develop' into luclu_fix-offline-startup-edge-case
frozeman Jan 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion modules/clientBinaryManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,9 @@ class Manager extends EventEmitter {
.then((localConfig) => {
if (!localConfig) {
log.info('No config for the ClientBinaryManager could be loaded, using local clientBinaries.json.');
localConfig = require('../clientBinaries.json');

const localConfigPath = path.join(Settings.userDataPath, 'clientBinaries.json');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont understand what is the fix. On line 87 it already tries to load the local config. Then later it will replace it with the remote loaded one, if one came if not it keeps the locally loaded one. On this line it basically already checked local and remote ones and therefore loads the one from the mist folder.

Copy link
Contributor Author

@luclu luclu Jan 12, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The statement in L#87 can only be reached if the computer is online. Otherwise it will simply return to the next promise-chain block. The skipped validation doesn't seem to be necessary as this did happened before as the clientBinaries.json was created.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as i understand this block:

// FETCH
 return got(BINARY_URL, {
             timeout: 3000,		             timeout: 3000,
             json: true,		             json: true,
         })		         })
// GET RESULT OR ERROR
         .then((res) => {		         .then((res) => {
             if (!res || _.isEmpty(res.body)) {		             if (!res || _.isEmpty(res.body)) {
                 throw new Error('Invalid fetch result');		                 throw new Error('Invalid fetch result');
             } else {		             } else {
                 return res.body;		                 return res.body;
             }		             }
         })		         })
// CATCH ERRORS from fetching, on or offline
         .catch((err) => {		         .catch((err) => {
             log.warn('Error fetching client binaries config from repo', err);		             log.warn('Error fetching client binaries config from repo', err);
         })		         })
// CONTINUE here, even if error was above thrown.
         .then((latestConfig) => {		         .then((latestConfig) => {

Please test this that a offline state will certainly not go into this then block after the catch and the duplication of the code is really necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's right. The next line will simply return and jump to the next block. As the localConfig is empty it will use my new code to fetch the already downloaded. If that one is not available (first run of Mist is offline) it uses the bundled. Those configs are both validated and so this plays nice with the omiting of the validation block.

localConfig = (fs.existsSync(localConfigPath)) ? require(localConfigPath) : require('../clientBinaries.json'); // eslint-disable-line no-param-reassign, global-require, import/no-dynamic-require, import/no-unresolved
}

// scan for node
Expand Down