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

Catch IOError in addition to OSError in AMS pull #104

Closed

Conversation

tofu-rocketry
Copy link
Member

This issue was fixed for the original style of messaging in #63 but didn't get carried over for AMS messaging. (There's a large section that should be factored out for both types of messaging to avoid issues like this!)

  • Catch IOError in addition to OSError in on_message as there is no real
    difference between them and we sometimes get the former being raised.
  • Rename 'error' to 'e' to match on_message convention to simplify
    refactoring later.

@tofu-rocketry tofu-rocketry added this to the 2.4.1 milestone Aug 5, 2019
@tofu-rocketry tofu-rocketry requested a review from a team as a code owner August 5, 2019 14:47
@tofu-rocketry tofu-rocketry self-assigned this Aug 5, 2019
jrha
jrha previously approved these changes Aug 5, 2019
@tofu-rocketry
Copy link
Member Author

Having seen this in operation, we need to sort out how we acknowledge messages we pull from AMS otherwise we end up in a loop of continually trying to pull down the same message.

- Catch IOError in addition to OSError in on_message as there is no real
  difference between them and we sometimes get the former being raised.
- Rename 'error' to 'e' to match on_message convention to simplify
  refactoring later.
@tofu-rocketry
Copy link
Member Author

Closing and noting issue in Issue #116.

@tofu-rocketry tofu-rocketry removed this from the 3.0.0 milestone Feb 4, 2020
@tofu-rocketry tofu-rocketry deleted the unhandled-ams-exception branch June 28, 2021 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants