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

Make rpc callback exception more explicit and wind up to show more infos #305

Merged

Conversation

unkcpz
Copy link
Member

@unkcpz unkcpz commented Dec 20, 2024

Combine with aiidateam/aiida-core#6671, it improves the debug workflow of daemon errors with RMQ.

@unkcpz unkcpz requested review from agoscinski and sphuber December 20, 2024 14:40
Copy link

@agoscinski agoscinski left a comment

Choose a reason for hiding this comment

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

Looks good to me moslty. I am not sure if the formatting of the error message as I find these raise ... from exc plus traceback hard to read because of the repetition but I also remember that I had to do this in some code because the original traceback wasn't always displayed. Was this here also the case or why do you print the traceback another time?

Here an example of the repetition I mean:

Traceback (most recent call last):
  File "/Users/alexgo/code/aiida-core/format_errmsg.py", line 10, in <module>
    callback()
  File "/Users/alexgo/code/aiida-core/format_errmsg.py", line 6, in foo
    faa()
  File "/Users/alexgo/code/aiida-core/format_errmsg.py", line 2, in faa
    raise ValueError("Something")
ValueError: Something

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

Traceback (most recent call last):
  File "/Users/alexgo/code/aiida-core/format_errmsg.py", line 27, in <module>
    raise RuntimeError(
RuntimeError: Error invoking callback 'foo' at /Users/alexgo/code/aiida-core/format_errmsg.py:5.
Exception: ValueError: Something

Full Traceback:
Traceback (most recent call last):
  File "/Users/alexgo/code/aiida-core/format_errmsg.py", line 10, in <module>
    callback()
  File "/Users/alexgo/code/aiida-core/format_errmsg.py", line 6, in foo
    faa()
  File "/Users/alexgo/code/aiida-core/format_errmsg.py", line 2, in faa
    raise ValueError("Something")
ValueError: Something

For the code

def faa():
    raise ValueError("Something")


def foo():
    faa()

callback = foo
try:
    callback()
except Exception as exc:
    import traceback
    import inspect

    # Get traceback as a string
    tb_str = ''.join(traceback.format_exception(type(exc), exc, exc.__traceback__))

    try:
        source_file = inspect.getfile(callback)
        # getsourcelines returns a tuple (list_of_source_lines, starting_line_number)
        _, start_line = inspect.getsourcelines(callback)
        callback_location = f'{source_file}:{start_line}'
    except Exception:
        callback_location = '<unknown location>'
    
    # Include the callback name, file/line info, and the full traceback in the message
    raise RuntimeError(
        f"Error invoking callback '{callback.__name__}' at {callback_location}.\n"
        f'Exception: {type(exc).__name__}: {exc}\n\n'
        f'Full Traceback:\n{tb_str}'
    ) from exc

Copy link

@agoscinski agoscinski left a comment

Choose a reason for hiding this comment

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

tests pass now and rebased

@unkcpz
Copy link
Member Author

unkcpz commented Jan 8, 2025

Was this here also the case or why do you print the traceback another time?

I can not remind myself when this traceback was swallowed, but yes it is a problem that the traceback may printed twice if the exception is came from the same thread where the runner runs. I think it is fine to get the merged since it helps for other PR where exceptions swallowed and make it hard to debug.

@unkcpz unkcpz merged commit ecef9b9 into aiidateam:master Jan 8, 2025
7 checks passed
@unkcpz unkcpz deleted the explicit-rpc-callback-exception-wind-up branch January 8, 2025 12:45
khsrali pushed a commit that referenced this pull request Jan 9, 2025
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.

2 participants