Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

trinity: Proxied services now record remote exceptions' tracebacks #1004

Merged
merged 1 commit into from
Jul 11, 2018

Conversation

gsalgado
Copy link
Contributor

@gsalgado gsalgado commented Jul 10, 2018

Closes: #956

@@ -144,6 +148,33 @@ def initialize_database(chain_config: ChainConfig, chaindb: AsyncChainDB) -> Non
)


class TracebackKeeper:
Copy link
Member

Choose a reason for hiding this comment

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

naming nitpick

maybe TracebackPreserver or TracebackRecorder?

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 on TracebackRecorder

if not inspect.ismethod(attr):
return attr

def wrapper(*args: Any, **kwargs: Any) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

maybe lift this out of the function body to the top level of the module and then this function body would be

if not inspect.ismethod(attr):
    return attr
else:
    return record_traceback_on_error(attr)

@gsalgado
Copy link
Contributor Author

I've improved this a bit based on this trick

It uses exception chaining, which means we don't need any extra hacks to special case the logging of those exceptions. This is how it looks like:

   ERROR  07-11 11:45:15     service  Unexpected error in <p2p.chain.FastChainSyncer object at 0x7f7fa2b3cf60>, exiting
trinity.chains.RemoteTraceback:
"""
Traceback (most recent call last):
  File "/home/salgado/src/py-evm/trinity/chains/__init__.py", line 176, in wrapper
    return attr(*args, **kwargs)
  File "/home/salgado/src/py-evm/evm/db/header.py", line 86, in get_canonical_block_hash
    validate_block_number(block_number, title="Block Number")
  File "/home/salgado/src/py-evm/evm/validation.py", line 201, in validate_block_number
    validate_is_integer(block_number, title="Block Number")
  File "/home/salgado/src/py-evm/evm/validation.py", line 37, in validate_is_integer
    "{title} must be a an integer.  Got: {0}".format(type(value), title=title)
evm.exceptions.ValidationError: Block Number must be a an integer.  Got: <class 'bytes'>
"""

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/salgado/src/py-evm/p2p/service.py", line 55, in run
    await self._run()
  File "/home/salgado/src/py-evm/p2p/chain.py", line 120, in _run
    await self.wait(self.db.coro_get_canonical_block_hash(b'foo'))
  File "/home/salgado/src/py-evm/p2p/cancel_token.py", line 102, in wait
    return await self.wait_first(awaitable, token=token, timeout=timeout)
  File "/home/salgado/src/py-evm/p2p/cancel_token.py", line 123, in wait_first
    return await wait_with_token(*awaitables, token=token_chain, timeout=timeout)
  File "/home/salgado/src/py-evm/p2p/cancel_token.py", line 160, in wait_with_token
    return done.pop().result()
  File "/home/salgado/src/py-evm/trinity/utils/mp.py", line 27, in method
    args,
  File "/usr/lib/python3.6/concurrent/futures/thread.py", line 56, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/usr/lib/python3.6/multiprocessing/managers.py", line 772, in _callmethod
    raise convert_to_error(kind, result)
evm.exceptions.ValidationError: Block Number must be a an integer.  Got: <class 'bytes'>

@gsalgado gsalgado changed the title wip: wrap trinity-proxied chain/db services with TracebackKeeper trinity: Proxied services now record remote exceptions' tracebacks Jul 11, 2018
The remote exception's traceback is then chained with the exception
instance sent to the main process, giving us a traceback containing the
stack frames that led to the exception in both processes.

Closes: ethereum#956
@gsalgado gsalgado merged commit 3a9583b into ethereum:master Jul 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants