-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Conversation
ping @nethip @vickramdhawal for review |
Bug: #14411 |
@niteskum Could you provide description here as to how the bugs has been fixed? And also possible reasons as to how one can run into this error? |
.done(function() { | ||
setUpdateStateInJSON("autoUpdateInProgress", flag); | ||
}); | ||
function nodeDomainInitialized(reset) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is this getting called from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK I see this is getting called at https://github.com/adobe/brackets/pull/14412/files#diff-d3624dc2cff4b4819b06f9e4a7407cf1R432
}) | ||
.fail(initState); | ||
} else { | ||
initState(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that if updateJsonHandler.parse()
fails, the code flow lands in initState
, which in turn is calling updateJsonHandler().parse
again. I think you can restucture the code for better readability as well as for proper code flow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restructured the Code. , Removed the CheckUpdateStatus() from initNode() function. Now initNode Function returns Promise . which is resolved when parsing is done. initNode is called and in done function checkUpdateStatus() function is called.
|
||
updateDomain.exec('initNode', { | ||
messageIds: MessageIds, | ||
updateDir: updateDir, | ||
requester: domainID | ||
}); | ||
|
||
updateDomain.on('data', receiveMessageFromNode); | ||
initState(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
initState()
not required anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now initState() is called from nodeDomainInitialized() function. so it is not required here.
@nethip In setupAutoUpdate() function initNode() and initState() function were called one after another. So there were no synchronization between setUpdateInProgressState() and CheckUpdateStatus() Changes in this PR: |
Auto Update Error Handlng Fix (adobe#14412)
* Auto Update Error Handlng Fix * Addressed Review comments
No description provided.