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

Handle invalid ABI retrieved from address_book gracefully #4606

Merged
merged 2 commits into from
Feb 20, 2017
Merged

Handle invalid ABI retrieved from address_book gracefully #4606

merged 2 commits into from
Feb 20, 2017

Conversation

jacogr
Copy link
Contributor

@jacogr jacogr commented Feb 20, 2017 •
edited
Loading

@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. M7-ui labels Feb 20, 2017
ngotchac
ngotchac approved these changes Feb 20, 2017
View reviewed changes
Copy link
Contributor

@ngotchac ngotchac left a comment

Choose a reason for hiding this comment

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

It does fix the issue. Might just want to print the actual corrupted ABI though.

js/src/ui/MethodDecoding/methodDecodingStore.js Outdated
try {
abi = new Abi(_abi);
} catch (error) {
console.warn('loadFromAbi', error);
Copy link
Contributor

Choose a reason for hiding this comment

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

Might want to print the _abi for debugging purposes.

@ngotchac ngotchac added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 20, 2017
@jacogr jacogr merged commit 3fc29b9 into master Feb 20, 2017
@jacogr jacogr deleted the jg-invalid-abi branch February 20, 2017 12:34
jacogr added a commit that referenced this pull request Feb 20, 2017
@jacogr
Handle invalid ABI retrieved from address_book gracefully (#4606)
5f9971a 
* Handle invalid ABI gracefully

* Also include failed abi in log
arkpar pushed a commit that referenced this pull request Feb 20, 2017
@jacogr @arkpar
Handle invalid ABI retrieved from address_book gracefully (#4606) (#4610
7df7024 
)

* Handle invalid ABI gracefully

* Also include failed abi in log
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ngotchac ngotchac ngotchac approved these changes

Assignees
No one assigned
Labels
A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jacogr @ngotchac