Fix unrecoverable connection state#3905
Conversation
|
Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset. In case there are security findings, they will be communicated to you as a comment inside the PR. Hope you’ll enjoy using Jit. Questions? Comments? Want to learn more? Get in touch with us. |
|
Hi @itssimon, thank you for your contribution! We will review it soon. |
redis/_parsers/base.py
Outdated
| async def can_read_destructive(self) -> bool: | ||
| if not self._connected: | ||
| raise RedisError("Buffer is closed.") | ||
| raise ConnectionError(SERVER_CLOSED_CONNECTION_ERROR) |
There was a problem hiding this comment.
I think that raising an OSError would be better here. This method is called in the AbstractConnection's can_read_destructive where OSError is caught and a ConnectionError is raised with additional info about the host and port.
There was a problem hiding this comment.
Makes sense. Updated.
Note that _AsyncHiredisParser raises ConnectionError for the same condition. Might want to update that too for consistency. Happy to do that in this PR as well.
There was a problem hiding this comment.
@itssimon yeah, it will be great, thank you!
Just use this error only for the can_read_destructive method.
on_connect should still raise ConnectionError as it is used in a different type of context.
2 participants
Description of change
Fixes #3904.
Pull Request check-list
Please make sure to review and check all of these items:
NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.