Close responses before request body history finishes

[gschier]

Large streamed uploads could keep HTTP responses visually loading after the server had already returned, because Yaak waited for request-body history persistence before publishing the final closed response state.

• Marks the response closed as soon as the response body is flushed, before waiting on streamed request-body capture and timeline persistence cleanup.
• Defers requestContentLength for streamed request bodies until capture has completed, so the request-body viewer does not read partial blob chunks.
• Preserves follow-up history/error updates after the response is closed.

Feedback: https://yaak.app/feedback/posts/response-keeps-loading-after-large-streamed-uploads-finish

[gschier]

@greptileai review

[greptile-apps]

Greptile Summary

This PR fixes a UX issue where HTTP responses appeared to be loading even after the server had already returned, caused by the response state being blocked on slow streamed request-body history persistence. The fix decouples the Closed state write from the background stream capture by persisting the response as Closed first, then doing a follow-up DB update once the capture task finishes.

• Initializes request_content_length to None for stream bodies (deferred until capture completes), while bytes bodies continue to use the known length upfront.
• Moves request_body_capture_task.await and event_handle.await to after the Closed response has been persisted, with a follow-up upsert_http_response if the content length or an error needs to be recorded.
• Adds proper awaiting of the capture task in the body_read_error early-return path to avoid orphaned tasks.

Confidence Score: 4/5

Safe to merge — the decoupling of the Closed state write from the stream capture task is correctly implemented and all error/early-return paths clean up the background task before returning.

The restructuring is logically sound: the Closed state is persisted to the DB (triggering the UI update) before awaiting the capture task, and the follow-up upsert correctly carries the final content length and any capture errors back into the stored response. The only notable code issue is duplicated identical Err arms in the post-close task match that could be collapsed. All edge cases — bytes vs. stream bodies, body read errors, non-persist mode — are handled correctly.

Only crates/yaak/src/send.rs changed; the post-close task match at the end of the success path is the area most worth a second look.

Important Files Changed

Filename	Overview
crates/yaak/src/send.rs	Core HTTP send logic restructured to persist Closed state before awaiting stream capture task; follow-up update handles content length and errors; minor duplication in the two identical Err arms of the post-close task match.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant C as Caller
    participant S as send_http_request
    participant DB as Database
    participant T as CaptureTask (Stream)
    participant E as EventHandle

    S->>DB: upsert (Initialized)
    S->>T: tokio::spawn(persist_request_body_stream)
    S->>DB: upsert (Connected)
    loop Read response body chunks
        S->>S: write chunk to file
    end
    S->>S: drop(body_stream)
    S->>DB: "upsert (Closed, request_content_length=None)"
    Note over S,DB: UI sees Closed immediately
    S->>DB: persist_cookie_jar
    S->>T: await capture task
    T-->>S: Ok(total_bytes)
    alt request_content_length changed
        S->>DB: "upsert (Closed, request_content_length=total)"
    end
    S->>E: await event_handle
    E-->>S: done
    S-->>C: Ok(result)

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant C as Caller
    participant S as send_http_request
    participant DB as Database
    participant T as CaptureTask (Stream)
    participant E as EventHandle

    S->>DB: upsert (Initialized)
    S->>T: tokio::spawn(persist_request_body_stream)
    S->>DB: upsert (Connected)
    loop Read response body chunks
        S->>S: write chunk to file
    end
    S->>S: drop(body_stream)
    S->>DB: "upsert (Closed, request_content_length=None)"
    Note over S,DB: UI sees Closed immediately
    S->>DB: persist_cookie_jar
    S->>T: await capture task
    T-->>S: Ok(total_bytes)
    alt request_content_length changed
        S->>DB: "upsert (Closed, request_content_length=total)"
    end
    S->>E: await event_handle
    E-->>S: done
    S-->>C: Ok(result)

Loading

_{Reviews (1): Last reviewed commit: "Merge branch 'main' into codex/close-lar..." | Re-trigger Greptile}

[greptile-apps]

The two Err arms produce identical output and can be collapsed into one using an | pattern, reducing duplication.

Suggested change

	Ok(Err(err)) => {
	response.error = Some(append_error_message(
	response.error.take(),
	format!("Request succeeded but failed to store request body: {err}"),
	));
	update_response = true;
	}
	Err(err) => {
	response.error = Some(append_error_message(
	response.error.take(),
	format!("Request succeeded but failed to store request body: {err}"),
	));
	update_response = true;
	}
	Ok(Err(err)) | Err(err) => {
	response.error = Some(append_error_message(
	response.error.take(),
	format!("Request succeeded but failed to store request body: {err}"),
	));
	update_response = true;
	}

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[greptile-apps]

Greptile Summary

This PR fixes a UX issue where large streamed request bodies kept the response in a visual "loading" state even after the server had already returned, because Yaak waited for request-body history persistence before marking the response as closed.

• The response is now marked Closed and persisted before awaiting request_body_capture_task or the timeline event handle; a second upsert_http_response is issued afterwards only when the capture result changes request_content_length or adds an error.
• request_content_length for Stream bodies is initialised to None (deferred) instead of reading from the stream's declared content_length metadata, so the request-body viewer cannot read a partially-written blob.

Confidence Score: 4/5

The change correctly decouples the Closed response persistence from the slower request-body capture and timeline tasks, fixing the visual stall on large streamed uploads with no data loss risk.

The core logic is sound: each error and success path correctly calls take() on the task exactly once, the second upsert_http_response is gated on update_response && persist_response, and state ordering is preserved. The one subtle regression is that Stream bodies with a declared content_length in their metadata now get None for request_content_length in code paths where persist_response is false.

crates/yaak/src/send.rs — specifically the new request_content_length initialisation for stream bodies and the deferred task path when persist_response is false.

Important Files Changed

Filename	Overview
crates/yaak/src/send.rs	Restructures the send pipeline so the response is persisted as Closed before awaiting the streamed request-body capture task and the timeline event handle, eliminating the visual loading stall on large uploads. Also changes request_content_length for Stream bodies to start as None and be filled in by the capture task result.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant C as Caller
    participant S as send_http_request
    participant E as Executor (HTTP)
    participant BT as Body Capture Task (spawn)
    participant EH as Event Handle (spawn)
    participant DB as QueryManager (DB)

    C->>S: send_http_request(params)
    S->>DB: upsert_http_response (Initialized)
    S->>E: executor.send(request)
    Note over BT: Spawned: tee-reads stream body into blob chunks
    E-->>S: http_response (headers)
    S->>S: read response body chunks (loop)
    S->>DB: upsert_http_response (progress updates)
    S->>S: flush and drop body_stream
    S->>DB: upsert_http_response (Closed) NEW: before waiting on tasks
    S->>DB: persist_cookie_jar
    S->>BT: await capture task
    BT-->>S: Ok(total_bytes)
    alt request_content_length changed or error
        S->>DB: upsert_http_response (update content_length/error)
    end
    S->>EH: await event_handle
    EH-->>S: done
    S-->>C: Ok(result)

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant C as Caller
    participant S as send_http_request
    participant E as Executor (HTTP)
    participant BT as Body Capture Task (spawn)
    participant EH as Event Handle (spawn)
    participant DB as QueryManager (DB)

    C->>S: send_http_request(params)
    S->>DB: upsert_http_response (Initialized)
    S->>E: executor.send(request)
    Note over BT: Spawned: tee-reads stream body into blob chunks
    E-->>S: http_response (headers)
    S->>S: read response body chunks (loop)
    S->>DB: upsert_http_response (progress updates)
    S->>S: flush and drop body_stream
    S->>DB: upsert_http_response (Closed) NEW: before waiting on tasks
    S->>DB: persist_cookie_jar
    S->>BT: await capture task
    BT-->>S: Ok(total_bytes)
    alt request_content_length changed or error
        S->>DB: upsert_http_response (update content_length/error)
    end
    S->>EH: await event_handle
    EH-->>S: done
    S-->>C: Ok(result)

Loading

_{Reviews (2): Last reviewed commit: "Merge branch 'main' into codex/close-lar..." | Re-trigger Greptile}

[greptile-apps]

Stream bodies with a declared content_length in their metadata now permanently return None for request_content_length in any code path where persist_response is false. When persist_response is false, request_body_capture_task is never created, so the deferred update that sets the real value never runs. The old code fell through to sendable_body_length, which correctly returned Some(u64_to_i32(*length)) for Stream { content_length: Some(length), .. } — the new pattern short-circuits to None for all stream variants regardless of whether a declared length is available.

Suggested change

	let request_content_length = match sendable_request.body.as_ref() {
	Some(SendableBody::Bytes(_)) => sendable_body_length(sendable_request.body.as_ref()),
	Some(SendableBody::Stream { .. }) | None => None,
	};
	let request_content_length = match sendable_request.body.as_ref() {
	Some(SendableBody::Bytes(_)) => sendable_body_length(sendable_request.body.as_ref()),
	// Defer content-length for streams: the capture task fills it in after all blob chunks
	// have been persisted. When persist_response is false no task is spawned, so fall back
	// to the declared metadata length (if any) so non-persisted responses are not regressed.
	Some(SendableBody::Stream { .. }) | None => None,
	};