Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -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.
1 change: 0 additions & 1 deletion logseq_sync/lib/effect_runner/eio/http_eio.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
68 changes: 67 additions & 1 deletion logseq_sync/test/transport_contract.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 ]
;;
Loading