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

Don't throw TimeoutError but rather the last error of retries #506

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nfx
Copy link
Contributor

@nfx nfx commented Jan 11, 2024

Changes

If we raise TimeoutError(..) from last_err, pytest displays the timeout error in the summary FAILED ...::test_running_real_assessment_job - TimeoutError: Timed out after 0:06:00, which makes troubleshooting harder during the stress time.

This change will put TimeoutError in the cause attribute of re-raised last error. See more at https://docs.python.org/3/library/exceptions.html#exception-context

Tests

  • make test run locally
  • make fmt applied
  • relevant integration tests applied

If we `raise TimeoutError(..) from last_err`, pytest displays the timeout error in the summary `FAILED ...::test_running_real_assessment_job - TimeoutError: Timed out after 0:06:00`, which makes troubleshooting harder during the stress time.

This change will put TimeoutError in the __cause__ attribute of re-raised last error. See more at https://docs.python.org/3/library/exceptions.html#exception-context
@jasongrout-db
Copy link

This seems like it reverses the conventional order of chained exceptions. Is this change just to accommodate pytest's printing? In general, I would suggest not changing the "correct" user-facing behavior to accommodate testing infrastructure.

It seems that pytest has had a lot of work recently in improving behavior and display of nested exceptions and exception groups. Perhaps the simplest is manually checking the cause, like in
this pytest issue, showing how to catch and test these types of exceptions. Would that help? Are you testing with the most recent pytest? I see there were very recent changes from late last year around exception handling.

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.

None yet

2 participants