Skip to content

Catch IOError in addition to OSError in AMS pull #104

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

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.

@tofu-rocketry tofu-rocketry modified the milestones: 2.4.1, 2.4.2 Sep 3, 2019
@tofu-rocketry tofu-rocketry force-pushed the unhandled-ams-exception branch from c4b4dae to dea7e72 Compare September 4, 2019 09:53
@tofu-rocketry tofu-rocketry force-pushed the unhandled-ams-exception branch from dea7e72 to 32771e6 Compare January 24, 2020 16:44
- 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 force-pushed the unhandled-ams-exception branch from 32771e6 to c88d0d9 Compare January 29, 2020 17:05
@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