Skip to content
This repository has been archived by the owner on Jun 30, 2022. It is now read-only.

Ignores socket errors that occur during shut_down #38

Merged

Conversation

alindeman
Copy link
Contributor

If the socket has already disconnected, e.g., we don't want to raise another error (see #34, #20) when trying to close it cleanly.

Scenario:

  1. The socket disconnects for an unexpected reason.
  2. An exception is raised when we try to write to it.
  3. The on_exception handler is called, which starts shutting the robot down.
  4. The shut_down method is called.
  5. By trying to close the socket cleanly, we attempt to write to it again, causing another exception (e.g., IOError or Errno::ECONNRESET)
  6. The new exception bubbles up and lita doesn't exit cleanly.

I think we should log, but not re-raise, exceptions during shut down of the connector.

WDYT @MattMencel @sjernigan @jimmycuadra

@jimmycuadra
Copy link
Collaborator

Good idea! It might be worth trying to determine which exceptions would actually occur in this scenario that we want to ignore rather than doing a blanket rescue. I've been thinking about this for Lita core as well—rescuing absolutely everything is pretty dangerous. Granted in this situation we're about to shut the program down anyway, but it still makes me nervous to be potentially ignoring critical errors from the system.

@alindeman
Copy link
Contributor Author

Good idea! It might be worth trying to determine which exceptions would actually occur in this scenario that we want to ignore rather than doing a blanket rescue. I've been thinking about this for Lita core as well—rescuing absolutely everything is pretty dangerous. Granted in this situation we're about to shut the program down anyway, but it still makes me nervous to be potentially ignoring critical errors from the system.

@jimmycuadra: Seems reasonable to me! If I do a bit more testing and flesh out the types of exceptions that occur in the scenario, is the PR otherwise acceptable? If so, I'll get right on that.

@jimmycuadra
Copy link
Collaborator

Yeah, absolutely. Thank you for your work!

If the socket has already disconnected, e.g., we don't want to raise _another_
error (see litaio#34, litaio#20) when trying to close it cleanly.
@alindeman alindeman force-pushed the alindeman/ignore-disconnect-errors branch from 987125c to f6ab143 Compare February 21, 2016 21:24
@alindeman
Copy link
Contributor Author

Yeah, absolutely. Thank you for your work!

@jimmycuadra No problem. I've updated the catch to be more specific.

jimmycuadra added a commit that referenced this pull request Feb 22, 2016
Ignores socket errors that occur during shut_down
@jimmycuadra jimmycuadra merged commit b72d524 into litaio:master Feb 22, 2016
@jimmycuadra
Copy link
Collaborator

Thanks, Andy! Hopefully this will help with the disconnection problem people have been having.

@jimmycuadra
Copy link
Collaborator

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants