From 997ba1c47fb6389aecb412fb49c2a612e0667547 Mon Sep 17 00:00:00 2001 From: rcmerci Date: Sun, 18 Oct 2026 21:01:57 +0800 Subject: [PATCH] Use HTTP/1.1 default connection semantics --- .../2026-10-18-http-default-connection.md | 42 ++++++++++++ logseq_sync/lib/effect_runner/eio/http_eio.ml | 1 - logseq_sync/test/transport_contract.ml | 68 ++++++++++++++++++- 3 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 docs/agent-guide/implemented/feature/2026-10-18-http-default-connection.md diff --git a/docs/agent-guide/implemented/feature/2026-10-18-http-default-connection.md b/docs/agent-guide/implemented/feature/2026-10-18-http-default-connection.md new file mode 100644 index 00000000..732ff492 --- /dev/null +++ b/docs/agent-guide/implemented/feature/2026-10-18-http-default-connection.md @@ -0,0 +1,42 @@ +# Default connection semantics for ordinary HTTP + +## Problem + +Ordinary requests explicitly send Connection close. A controlled iPhone comparison reproduced truncated responses with this legal header and complete responses with explicit keep-alive, including reversed request order. This isolates a trigger, not a proven server, VPN or TLS root cause. The user requests omitting the header and a minimal PR. + +## Decision + +Remove only the unconditional Connection close entry from ordinary Http_eio request headers, relying on HTTP1.1 defaults. Keep per-attempt TCP/TLS setup and owner retirement after response completion. Do not introduce pooling or change DNS, TLS, proxy, buffers, deadlines, retries, dependencies or WebSocket Upgrade. + +Header serialization and socket lifetime are transport-owned: Core events cannot execute either, and injecting a completion would not reproduce them. Extend existing public Runner/independent TLS-peer tests to inspect actual ordinary GET/PUT headers while retaining full-body, truncated-body and resource-retirement checks. Assert WebSocket Upgrade headers remain present and sequential requests through one Runner still retire separate connections. No test bypasses public interfaces or adds a duplicate lower-level suite. + +This clean branch is based on current origin/main. It excludes all earlier diagnostic switches, Swift/C instrumentation, temporary fixtures and other fixes. Device build1.5877.47 contained the same ordinary header change in the prior diagnostic checkout; normal ready and actual WebSocket were observed. It is not the exact clean PR binary, full cross-ABI validation is not established, and manual initial graph-download acceptance remains pending. Do not publish private logs, graph contents, identifiers or credentials. + +## Alternatives considered + +### Explicit keep-alive or a connection pool + +The user requests omission only. Pooling would change ownership beyond scope. + +## Acceptance criteria + +- Existing real-wire GET/PUT tests demonstrate the prior header-presence RED and pass after omission. +- Complete framing, truncation rejection, Upgrade headers and attempt-owned cleanup retain coverage. +- Final diff contains only the header deletion, necessary tests and this repository-required decision; independent review and exact-head CI are recorded. +- Publish a non-draft PR without merging or claiming an identified external root cause. + +## Risks + +- This mitigation does not identify the component responsible for the observed premature close. +- Each attempt still closes its connection; no cross-request reuse is introduced. +- Close-delimited bodies still need read EOF or the existing timeout. + +## Questions + +None. The user explicitly authorizes submission of this minimal PR. + +## Consequences + +The production change is one deleted header entry. The final clean-source build and sync suite pass with the original locked host providers: 313 reported cases, including all 139 transport cases. Before deletion, 16 ordinary GET/download checks, five PUT checks and the same-Runner sequence failed on the actual captured Connection header; existing WebSocket Upgrade checks passed. Independent read-only review against main 21dbe01 found no evidence-backed issue. Exact-head CI is required after publication; manual real-graph initialization remains pending. + +This document validates. Full decision-document validation reports only the three unchanged historical documents already missing required sections; unrelated documents are outside this PR. diff --git a/logseq_sync/lib/effect_runner/eio/http_eio.ml b/logseq_sync/lib/effect_runner/eio/http_eio.ml index 9759b58b..ce420535 100644 --- a/logseq_sync/lib/effect_runner/eio/http_eio.ml +++ b/logseq_sync/lib/effect_runner/eio/http_eio.ml @@ -173,7 +173,6 @@ let with_attempt ~sw ~authenticator ~network ~clock ~response_budget request ~co let headers = Httpun.Headers.of_list (("host", host_header) - :: ("connection", "close") :: request_headers) in let target = Uri.path_and_query request.uri in diff --git a/logseq_sync/test/transport_contract.ml b/logseq_sync/test/transport_contract.ml index 26b7df50..8441a4a2 100644 --- a/logseq_sync/test/transport_contract.ml +++ b/logseq_sync/test/transport_contract.ml @@ -567,6 +567,33 @@ let check_retired report = rows ;; +let captured_request_headers row = + let encoded = Yojson.Basic.Util.(row |> member "request" |> to_string) in + let request = + String.init (String.length encoded / 2) (fun n -> + Char.chr (int_of_string ("0x" ^ String.sub encoded (2 * n) 2))) + in + let rec headers = function + | [] | "" :: _ -> [] + | line :: rest -> line :: headers rest + in + String.split_on_char '\n' request + |> List.tl + |> List.map (fun line -> String.lowercase_ascii (String.trim line)) + |> headers +;; + +let check_default_connection report = + Yojson.Basic.Util.to_list report + |> List.iter (fun row -> + Alcotest.(check bool) + "ordinary request omits Connection" + false + (List.exists + (String.starts_with ~prefix:"connection:") + (captured_request_headers row))) +;; + let http_test ~download ~success @@ -623,7 +650,8 @@ let http_test (read_file (Filename.concat staging name)))); Runner.shutdown t) in - check_retired report + check_retired report; + check_default_connection report ;; let frame ?(fin = true) opcode payload = @@ -708,6 +736,13 @@ let ws_test ?(extra = "") ?(segment = 16384) ~messages ~terminal wire () = Runner.shutdown t) in check_retired report; + Yojson.Basic.Util.to_list report + |> List.iter (fun row -> + let headers = captured_request_headers row in + Alcotest.(check bool) "WebSocket keeps Connection Upgrade" true + (List.mem "connection: upgrade" headers); + Alcotest.(check bool) "WebSocket keeps Upgrade websocket" true + (List.mem "upgrade: websocket" headers)); if not terminal then ( let frames = @@ -1917,6 +1952,7 @@ let asset_upload ~status ~refresh () = Runner.shutdown runner) in check_retired report; + check_default_connection report; Yojson.Basic.Util.to_list report |> List.iter (fun row -> let encoded = Yojson.Basic.Util.(row |> member "request" |> to_string) in @@ -4214,3 +4250,33 @@ let scenarios = {|{"graphs":[{"graph-id":"11111111-1111-4111-8111-111111111111","graph-name":null,"graph-ready-for-use?":true,"future":true}]}|}) ] ;; + +let test_default_connection_sequence () = + let wire = + response ~headers:"Content-Length: 3\r\nConnection: keep-alive\r\n" "abc" + in + let report = + with_peer [ case wire; case wire ] (fun ~environment ~sw ~support ~origin -> + let posted = ref [] and invalidations = ref 0 in + let t = runner ~environment ~sw ~support ~posted ~invalidations () in + Fun.protect ~finally:(fun () -> Runner.shutdown t) (fun () -> + List.iter (fun download -> + posted := []; + Runner.submit t (operation ~download origin); + wait (Eio.Stdenv.clock environment) (fun () -> + Option.is_some (completed !posted)); + Alcotest.(check bool) "response completes without waiting for peer EOF" true + (Result.is_ok (Option.get (completed !posted)))) + [ false; true ])) + in + Alcotest.(check int) "same Runner opens two separate connections" 2 + (List.length (Yojson.Basic.Util.to_list report)); + check_retired report; + check_default_connection report +;; + +let scenarios = + scenarios + @ [ Alcotest.test_case "HTTP default connection retires between requests" + `Quick test_default_connection_sequence ] +;;