Skip to content

Commit

Permalink
Skip state update upon failure (#8432)
Browse files Browse the repository at this point in the history 
Many of the "message manager" background methods return a full copy of
the background state in their response; presumably to save us from
making a full round-trip to update the UI `metamask` state after it
changes. However, the action creators responsible for calling these
methods were calling `updateMetamaskState` even when the background
method failed. In effect, they were setting the UI `metamask` state to
`undefined`.

They have been updated to only set the UI `metamask` state if the
background method succeeded.
  • Loading branch information
@Gudahtt
Gudahtt authored Apr 27, 2020
1 parent 3ab00b4 commit b58c0d7
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 14 deletions.
3 changes: 0 additions & 3 deletions test/unit/ui/app/actions.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,6 @@ describe('Actions', function () {
})
const expectedActions = [
{ type: 'SHOW_LOADING_INDICATION', value: undefined },
{ type: 'UPDATE_METAMASK_STATE', value: undefined },
{ type: 'HIDE_LOADING_INDICATION' },
{ type: 'DISPLAY_WARNING', value: 'error' },
]
Expand Down Expand Up @@ -651,7 +650,6 @@ describe('Actions', function () {
const store = mockStore()
const expectedActions = [
{ type: 'SHOW_LOADING_INDICATION', value: undefined },
{ type: 'UPDATE_METAMASK_STATE', value: undefined },
{ type: 'HIDE_LOADING_INDICATION' },
{ type: 'DISPLAY_WARNING', value: 'error' },
]
Expand Down Expand Up @@ -741,7 +739,6 @@ describe('Actions', function () {
const store = mockStore({ metamask: devState })
const expectedActions = [
{ type: 'SHOW_LOADING_INDICATION', value: undefined },
{ type: 'UPDATE_METAMASK_STATE', value: undefined },
{ type: 'HIDE_LOADING_INDICATION' },
{ type: 'DISPLAY_WARNING', value: 'error' },
]
Expand Down
22 changes: 11 additions & 11 deletions ui/app/store/actions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,6 @@ export function signMsg (msgData) {
log.debug(`actions calling background.signMessage`)
background.signMessage(msgData, (err, newState) => {
log.debug('signMessage called back')
dispatch(updateMetamaskState(newState))
dispatch(hideLoadingIndication())

if (err) {
Expand All @@ -449,6 +448,7 @@ export function signMsg (msgData) {
return reject(err)
}

dispatch(updateMetamaskState(newState))
dispatch(completedTx(msgData.metamaskId))
dispatch(closeCurrentNotificationWindow())

Expand All @@ -466,7 +466,6 @@ export function signPersonalMsg (msgData) {
log.debug(`actions calling background.signPersonalMessage`)
background.signPersonalMessage(msgData, (err, newState) => {
log.debug('signPersonalMessage called back')
dispatch(updateMetamaskState(newState))
dispatch(hideLoadingIndication())

if (err) {
Expand All @@ -475,6 +474,7 @@ export function signPersonalMsg (msgData) {
return reject(err)
}

dispatch(updateMetamaskState(newState))
dispatch(completedTx(msgData.metamaskId))
dispatch(closeCurrentNotificationWindow())

Expand All @@ -491,14 +491,14 @@ export function decryptMsgInline (decryptedMsgData) {
log.debug(`actions calling background.decryptMessageInline`)
background.decryptMessageInline(decryptedMsgData, (err, newState) => {
log.debug('decryptMsgInline called back')
dispatch(updateMetamaskState(newState))

if (err) {
log.error(err)
dispatch(displayWarning(err.message))
return reject(err)
}

dispatch(updateMetamaskState(newState))
decryptedMsgData = newState.unapprovedDecryptMsgs[decryptedMsgData.metamaskId]
return resolve(decryptedMsgData)
})
Expand All @@ -514,7 +514,6 @@ export function decryptMsg (decryptedMsgData) {
log.debug(`actions calling background.decryptMessage`)
background.decryptMessage(decryptedMsgData, (err, newState) => {
log.debug('decryptMsg called back')
dispatch(updateMetamaskState(newState))
dispatch(hideLoadingIndication())

if (err) {
Expand All @@ -523,6 +522,7 @@ export function decryptMsg (decryptedMsgData) {
return reject(err)
}

dispatch(updateMetamaskState(newState))
dispatch(completedTx(decryptedMsgData.metamaskId))
dispatch(closeCurrentNotificationWindow())
console.log(decryptedMsgData)
Expand All @@ -540,7 +540,6 @@ export function encryptionPublicKeyMsg (msgData) {
log.debug(`actions calling background.encryptionPublicKey`)
background.encryptionPublicKey(msgData, (err, newState) => {
log.debug('encryptionPublicKeyMsg called back')
dispatch(updateMetamaskState(newState))
dispatch(hideLoadingIndication())

if (err) {
Expand All @@ -549,6 +548,7 @@ export function encryptionPublicKeyMsg (msgData) {
return reject(err)
}

dispatch(updateMetamaskState(newState))
dispatch(completedTx(msgData.metamaskId))
dispatch(closeCurrentNotificationWindow())

Expand All @@ -566,7 +566,6 @@ export function signTypedMsg (msgData) {
log.debug(`actions calling background.signTypedMessage`)
background.signTypedMessage(msgData, (err, newState) => {
log.debug('signTypedMessage called back')
dispatch(updateMetamaskState(newState))
dispatch(hideLoadingIndication())

if (err) {
Expand All @@ -575,6 +574,7 @@ export function signTypedMsg (msgData) {
return reject(err)
}

dispatch(updateMetamaskState(newState))
dispatch(completedTx(msgData.metamaskId))
dispatch(closeCurrentNotificationWindow())

Expand Down Expand Up @@ -894,13 +894,13 @@ export function cancelMsg (msgData) {
dispatch(showLoadingIndication())
return new Promise((resolve, reject) => {
background.cancelMessage(msgData.id, (err, newState) => {
dispatch(updateMetamaskState(newState))
dispatch(hideLoadingIndication())

if (err) {
return reject(err)
}

dispatch(updateMetamaskState(newState))
dispatch(completedTx(msgData.id))
dispatch(closeCurrentNotificationWindow())

Expand All @@ -916,13 +916,13 @@ export function cancelPersonalMsg (msgData) {
return new Promise((resolve, reject) => {
const id = msgData.id
background.cancelPersonalMessage(id, (err, newState) => {
dispatch(updateMetamaskState(newState))
dispatch(hideLoadingIndication())

if (err) {
return reject(err)
}

dispatch(updateMetamaskState(newState))
dispatch(completedTx(id))
dispatch(closeCurrentNotificationWindow())

Expand All @@ -938,13 +938,13 @@ export function cancelDecryptMsg (msgData) {
return new Promise((resolve, reject) => {
const id = msgData.id
background.cancelDecryptMessage(id, (err, newState) => {
dispatch(updateMetamaskState(newState))
dispatch(hideLoadingIndication())

if (err) {
return reject(err)
}

dispatch(updateMetamaskState(newState))
dispatch(completedTx(id))
dispatch(closeCurrentNotificationWindow())

Expand All @@ -960,13 +960,13 @@ export function cancelEncryptionPublicKeyMsg (msgData) {
return new Promise((resolve, reject) => {
const id = msgData.id
background.cancelEncryptionPublicKey(id, (err, newState) => {
dispatch(updateMetamaskState(newState))
dispatch(hideLoadingIndication())

if (err) {
return reject(err)
}

dispatch(updateMetamaskState(newState))
dispatch(completedTx(id))
dispatch(closeCurrentNotificationWindow())

Expand All @@ -982,13 +982,13 @@ export function cancelTypedMsg (msgData) {
return new Promise((resolve, reject) => {
const id = msgData.id
background.cancelTypedMessage(id, (err, newState) => {
dispatch(updateMetamaskState(newState))
dispatch(hideLoadingIndication())

if (err) {
return reject(err)
}

dispatch(updateMetamaskState(newState))
dispatch(completedTx(id))
dispatch(closeCurrentNotificationWindow())

Expand Down

0 comments on commit b58c0d7

Please sign in to comment.