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

Uaerr #1669

Closed
wants to merge 3 commits into from
Closed

Uaerr #1669

wants to merge 3 commits into from

Conversation

oroulet
Copy link
Member

@oroulet oroulet commented Jul 1, 2024

No description provided.

pass


class UaClientConnectionLostError(UaError):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe have this also inherit ConnectionError for any existing code out there that excepts that?

@@ -531,13 +532,22 @@ async def check_connection(self) -> None:
# if not it throws the underlying exception
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change makes this comment wrong

Comment on lines 533 to +544
if self._renew_channel_task is not None:
if self._renew_channel_task.done():
await self._renew_channel_task
try:
await self._renew_channel_task
except Exception as ex:
raise UaClientRenewChannelError() from ex
if self._monitor_server_task is not None:
if self._monitor_server_task.done():
await self._monitor_server_task
try:
await self._monitor_server_task
except Exception as ex:
raise UaClientConnectionLostError() from ex
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe change the order of these two blocks. If the monitor task determines that the connection is lost, any errors in the renew channel task is most likely due to the lost connection?

@oroulet oroulet closed this Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants