fix(truapi-provider): track tunnel liveness by flag, not by dialling - #445
Merged
Conversation
`ensure_tunnel` decided whether a cached tunnel was still alive by connecting to its loopback port. That connection is itself accepted and relayed, so every reuse cost an upstream TLS handshake to a public bootnode, and the blocking connect ran while the tunnel map was locked. The accept loop now carries a flag it clears on exit, including on panic, and the cache hit reads that instead. `AliveUntilDropped` owns the clearing so the three early-return setup failures and an unwind all report the same way. Also stop `redacted` from returning a URL with its credentials intact: both setters reject a cannot-be-a-base URL, and serializing the clone anyway emitted exactly what the function exists to remove. It now falls back to scheme and host. Covers the tunnel lifecycle, which had no tests: reuse, replacement of a dead tunnel, and that distinct hosts and distinct ports are distinct upstreams. Plus redaction of a username, a username with a password, and the unchanged no-userinfo case.
filvecchiato
approved these changes
Aug 18, 2026
…le branch An adversarial review found the previous commit's own contribution untested: gutting `AliveUntilDropped` left every test passing, because the dead-tunnel test set the flag by hand instead of driving the guard. `the_guard_clears_the_flag_when_the_loop_ends` runs the guard on a real thread and fails when it is removed. The dead-tunnel tests assert the replacement is live rather than that its port differs, since a genuinely dead tunnel releases its port and the kernel may hand the same one back. `redacted` drops its fallback branch: both setters reject only a URL with no host, an empty domain, or the `file` scheme, none of which can carry the userinfo that reaches that code, so the branch was unreachable and its comment named the wrong condition. The guard's doc no longer claims panic coverage, which the release profile's `panic = "abort"` does not provide, and `run_tunnel` no longer describes an `ensure_tunnel` probe this branch removed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #276.
ensure_tunneldecides whether a cached tunnel is still usable by reading a flag the accept loop clears when it exits. Nothing dials the loopback port to find out, so reusing a tunnel costs no upstream TLS handshake and the tunnel map isnever held across a blocking connect.
AliveUntilDroppedowns the clearing, so a setup failure and a panic both report the same way.redactedfalls back to scheme and host whenset_usernameorset_passwordreject the URL, so a cannot-be-a-base URL cannot print the credentials the function exists to strip.Tests cover the tunnel lifecycle: a live tunnel is reused, a dead one is rebound, and distinct hosts and distinct ports are distinct upstreams. Redaction is covered for a username, a username with a password, and a URL with no userinfo.
The relay path itself is still untested. That needs a local TLS stub server, so it belongs in its own PR.