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

Swallowing the CommClosedError when the stream handler closes may remove relevant exception information #8683

Open
hendrikmakait opened this issue Jun 7, 2024 · 0 comments

Comments

@hendrikmakait
Copy link
Member

In

except CommClosedError:
closed = True
logger.info(
"Connection to %s has been closed.",
comm.peer_address,
)
break

we always swallow the CommClosedError without even logging it. This can be problematic because we use this error both for the happy and the sad path of stream handler closing. For example, in convert_stream_closed_error (

def convert_stream_closed_error(obj, exc):
"""
Re-raise StreamClosedError as CommClosedError.
"""
if exc.real_error is not None:
# The stream was closed because of an underlying OS error
exc = exc.real_error
if isinstance(exc, ssl.SSLError):
if exc.reason and "UNKNOWN_CA" in exc.reason:
raise FatalCommClosedError(f"in {obj}: {exc.__class__.__name__}: {exc}")
raise CommClosedError(f"in {obj}: {exc.__class__.__name__}: {exc}") from exc
else:
raise CommClosedError(f"in {obj}: {exc}") from exc
), we may attach relevant exception information to the CommClosedError but will subsequently ignore it in handle_stream.

Ideally, we could differentiate between when stream handler closing is expected and when it's unexpected and log more information in the latter case.

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

No branches or pull requests

1 participant