[yungwine]

No description provided.

[Copilot]

Pull request overview

This PR implements safer connection close handling for the LiteClient to prevent resource leaks and race conditions during shutdown. The changes add defensive patterns like re-entrance guards, proper exception handling in the listener loop, and graceful task cancellation with timeouts.

Key changes:

• Added comprehensive exception handling in the listen() method with proper task cleanup
• Implemented re-entrance protection and safer task cancellation in the close() method using timeouts and shields
• Moved handshake await earlier in connect() to ensure connection is established before starting background tasks

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

[Copilot]

Commented-out code should be removed. If fut.cancel() is intentionally not used in favor of set_exception(), add a comment explaining why cancellation is not used (e.g., to provide a more informative error message to callers).

Suggested change

	# fut.cancel()
	# We use set_exception instead of fut.cancel() to provide a more informative error message to callers.

[Copilot]

Using asyncio.shield() with await on a cancelled task will still propagate CancelledError. The shield protects the inner task from outer cancellation, but doesn't suppress exceptions from the awaited task itself. Since the task is already cancelled on line 249, this will always raise CancelledError, which is then caught and suppressed on line 253. Consider removing the shield here as it serves no purpose, or if the intent is to protect from external cancellation, shield the entire close operation instead.

Suggested change

	await asyncio.shield(asyncio.wait_for(task, timeout=1.0))
	await asyncio.wait_for(task, timeout=1.0)

[Copilot]

The shield here doesn't provide meaningful protection since you're already in a suppress(Exception) block and there's no outer cancellation to protect against at this point. The shield would only be useful if close() itself could be cancelled, but the _closing guard prevents re-entrance. Consider removing the shield or moving it to wrap the entire close operation if the intent is to make close() non-cancellable.

Suggested change

	await asyncio.shield(asyncio.wait_for(w.wait_closed(), timeout=1.0))
	await asyncio.wait_for(w.wait_closed(), timeout=1.0)

[Copilot]

[nitpick] These two separate suppress blocks could be combined into one to reduce nesting and improve readability, especially since they both suppress all exceptions. Consider: with suppress(Exception): w.close(); await asyncio.wait_for(w.wait_closed(), timeout=1.0)

Suggested change

	with suppress(Exception):
	await asyncio.shield(asyncio.wait_for(w.wait_closed(), timeout=1.0))
	await asyncio.shield(asyncio.wait_for(w.wait_closed(), timeout=1.0))

[Copilot]

If self.writer is None (e.g., connection was never established or already closed), this will raise AttributeError when calling w.close() on line 263. Add a null check: if w is not None: before attempting to close.

Suggested change

	with suppress(Exception):
	w.close()
	with suppress(Exception):
	await asyncio.shield(asyncio.wait_for(w.wait_closed(), timeout=1.0))
	if w is not None:
	with suppress(Exception):
	w.close()
	with suppress(Exception):
	await asyncio.shield(asyncio.wait_for(w.wait_closed(), timeout=1.0))

[Copilot]

'except' clause does nothing but pass and there is no explanatory comment.

Suggested change

	except (asyncio.CancelledError, Exception):
	except (asyncio.CancelledError, Exception):
	# Suppress exceptions during task cancellation and awaiting,
	# as they are expected during shutdown and can be safely ignored.