Skip to content

fix(sdk): remove REST tunnel bridge copy #985

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.

Sign up for GitHub

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

Closed
G4614 wants to merge 2 commits into from
Closed

fix(sdk): remove REST tunnel bridge copy #985

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1c4e3ba
fix(sdk): preserve tunnel half-close responses
G4614 Jul 23, 2026
889729e
test(sdk): mark cloud tunnel half-close e2e xfail
G4614 Jul 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions scripts/test/e2e/cases/test_node_tunnel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,9 @@ def test_node_sdk_tunnel_rejects_stopped_box(node_tunnel_env):
env.pop("BOXLITE_E2E_SKIP_STOPPED_BOX")
_assert_node_tunnel_passes(_run_node_tunnel(env))


@pytest.mark.xfail(
strict=True,
reason="TCP half-close currently drops the guest response",
reason="cloud tunnel path through NLB TLS termination does not guarantee TCP half-close",

@coderabbitai coderabbitai Bot Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not retain strict xfails for the fixed half-close coverage. Both tests will fail as XPASS when the tunnel correctly preserves half-close responses, defeating the PR’s required E2E validation.

  • scripts/test/e2e/cases/test_node_tunnel.py#L83-L83: remove the strict xfail or scope it only to NLB deployments.
  • scripts/test/e2e/cases/test_sdk_tunnel.py#L229-L229: remove the strict xfail or scope it only to NLB deployments.
📍 Affects 2 files
  • scripts/test/e2e/cases/test_node_tunnel.py#L83-L83 (this comment)
  • scripts/test/e2e/cases/test_sdk_tunnel.py#L229-L229
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/test/e2e/cases/test_node_tunnel.py` at line 83, Remove the strict
xfail from the half-close coverage in
scripts/test/e2e/cases/test_node_tunnel.py:83-83 and
scripts/test/e2e/cases/test_sdk_tunnel.py:229-229, or condition each xfail
exclusively on NLB deployments; preserve the tests as required E2E validation
for other tunnel configurations.

)
def test_node_sdk_tunnel_preserves_tcp_half_close(node_tunnel_env):
env = {**node_tunnel_env}
Expand Down
3 changes: 1 addition & 2 deletions scripts/test/e2e/cases/test_sdk_tunnel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,10 +224,9 @@ async def test_python_sdk_tunnel_rejects_stopped_box(rt, image):
return
assert SERVICES[0][1] not in stopped_response


@pytest.mark.xfail(
strict=True,
reason="TCP half-close currently drops the guest response",
reason="cloud tunnel path through NLB TLS termination does not guarantee TCP half-close",
)
@pytest.mark.asyncio
async def test_python_sdk_tunnel_preserves_tcp_half_close(rt, image):
Expand Down
25 changes: 13 additions & 12 deletions src/boxlite/src/rest/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ impl ApiClient {
pub(crate) async fn connect_box_network_tunnel(
&self,
uri: &str,
) -> BoxliteResult<tokio::io::DuplexStream> {
) -> BoxliteResult<hyper_util::rt::TokioIo<hyper::upgrade::Upgraded>> {
use http_body_util::Empty;
use hyper::{Method, Request, Uri};
use hyper_util::rt::TokioIo;
Expand Down Expand Up @@ -399,12 +399,7 @@ impl ApiClient {
let upgraded = hyper::upgrade::on(response)
.await
.map_err(|e| BoxliteError::Network(format!("CONNECT upgrade failed: {e}")))?;
let (local, mut pump_end) = tokio::io::duplex(64 * 1024);
let mut remote = TokioIo::new(upgraded);
tokio::spawn(async move {
let _ = tokio::io::copy_bidirectional(&mut pump_end, &mut remote).await;
});
Ok(local)
Ok(TokioIo::new(upgraded))
}

/// Prepare a box service tunnel and return its public descriptor.
Expand Down Expand Up @@ -677,19 +672,25 @@ mod tests {
let mut payload = [0; 4];
socket.read_exact(&mut payload).await.unwrap();
assert_eq!(&payload, b"ping");
socket.write_all(&payload).await.unwrap();
assert_eq!(
socket.read_u8().await.unwrap_err().kind(),
std::io::ErrorKind::UnexpectedEof
);
socket.write_all(b"pong").await.unwrap();
});

let client =
ApiClient::new(&BoxliteRestOptions::new(format!("http://127.0.0.1:{port}"))).unwrap();
let mut stream = client
let stream = client
.connect_box_network_tunnel(&format!("http://127.0.0.1:{port}"))
.await
.unwrap();
stream.write_all(b"ping").await.unwrap();
let (mut reader, mut writer) = tokio::io::split(stream);
writer.write_all(b"ping").await.unwrap();
writer.shutdown().await.unwrap();
let mut response = [0; 4];
stream.read_exact(&mut response).await.unwrap();
assert_eq!(&response, b"ping");
reader.read_exact(&mut response).await.unwrap();
assert_eq!(&response, b"pong");
server.await.unwrap();
}

Expand Down
Loading