search

omgnetwork / plasma-mvp

OmiseGO's research implementation of Minimal Viable Plasma
MIT License
561 stars 158 forks source link

started adding some error handling for child chain service #130

Closed rcallan closed 6 years ago

rcallan commented 6 years ago

this should fix #99. the invalid requests no longer returns a KeyError, and instead passes the error back to the CLI. please feel free to let me know if you have any comments or questions

smartcontracts commented 6 years ago

Awesome, thanks for the PR! I think we should add the error handling check to each call the CLI makes to the client. Might be easier with a helper?

def client_call(fn, args=()):
    try:
        fn(*args)
    except ChildChainServiceError as err:
        ...

and then

client_call(client.apply_transaction, args=(tx))

Not sure if that ends up being cleaner or more confusing than try/except on each... Need to think about it

rcallan commented 6 years ago

a helper function seems like a good way to go. I added one similar to what you mentioned, and so far it's just being used for the client calls that call the child chain service

smartcontracts commented 6 years ago

Awesome, LGTM. Approving.

DavidKnott commented 6 years ago

@rcallan Would you mind rebasing?

rcallan commented 6 years ago

okay sure, I just did a rebase

smartcontracts commented 6 years ago

Merged, thanks!