Skip to content

Improve NetworkTransport and add test coverage #108

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

Merged
merged 5 commits into from
May 7, 2025

Conversation

mattt
Copy link
Contributor

@mattt mattt commented May 7, 2025

In my experience, the existing NetworkTransport implementation hasn't been super reliable. In iMCP, users have observed random disconnections, connection hangs, and inconsistent behavior during network transitions.

This PR implements a robust connection management system to address these reliability issues:

  • Heartbeat Mechanism ❤️: Regularly checks connection health to detect and recover from "zombie" connections 1
  • Automatic Reconnection: Implements smart reconnection with exponential backoff to handle temporary network issues
  • Improved Error Handling: Better detection and recovery from various network failure scenarios
  • Connection State Management: More reliable tracking of connection state and proper cleanup

This PR also adds test coverage to NetworkTransport, which is a big win. Getting that to work was a bit tedious, as it required defining an internal protocol to match the NWConnection (final class). But I suppose it wasn't any more onerous than the contortions we needed to mock the Foundation URL Loading System 🥲

Anyway, with these changes, local builds of iMCP are working much more consistently

Footnotes

  1. I suppose we could use Ping for this, but it seemed easier to drop down from the protocol layer to the transport layer for liveness checks.

@mattt mattt merged commit 87f33d0 into main May 7, 2025
6 checks passed
@mattt mattt deleted the mattt/fix-network-transport branch May 7, 2025 17:36
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.

1 participant