[PR #41] [MERGED] refactor(polling): centralize PollingFallback stream-drain into one helper #43

New issue

Closed

opened 2026-10-26 22:34:19 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:34:19 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-swift-sdk/pull/41
Author: @mattmillerai
Created: 7/11/2026
Status: ✅ Merged
Merged: 7/16/2026
Merged by: @mattmillerai

Base: main ← Head: matt/be-2855-pollingfallback-drain-helper

📝 Commits (1)

4e1e915 refactor(polling): centralize PollingFallback stream-drain into one helper

📊 Changes

3 files changed (+58 additions, -68 deletions)

View changed files

📝 Sources/ComfySwiftSDK/Internal/PollingFallback.swift (+47 -0)
📝 Sources/ComfySwiftSDK/Internal/ReattachCoordinator.swift (+7 -24)
📝 Sources/ComfySwiftSDK/Internal/WebSocketSession.swift (+4 -44)

📄 Description

ELI-5

When a job is running, the SDK normally streams progress over a WebSocket. If that socket drops, the SDK "falls back" to polling: it repeatedly asks the server for status and re-emits those updates to the caller. Two different places did this same fall-back: the WebSocket-drop handoff and the reattach path (resuming a job after the app was backgrounded).

Both places had the exact same block of code — build the poller, forward every event, and know which events mean "the job is over" so it can close the stream exactly once. Copy-pasted logic like that tends to drift: fix a bug in one and forget the other. This PR pulls that block into a single shared helper both places call, so the "which events are terminal + close-once" contract lives in one spot.

What changed

Added PollingFallback.drain(into:transport:jobId:startTime:clock:lastEmittedPhase:lastEmittedFraction:hasEmittedQueued:hasEmittedFinalizing:) — a static forwarder that builds a PollingFallback, drains its eventStream into the passed continuation, and applies the terminal-event contract (yield every event; finish() once on the first .complete/.failed/.cancelled; finish() on natural stream end; wrap any thrown error as .failed(.unknown(underlying:)) then finish).
WebSocketSession: the transient-error handoff now calls PollingFallback.drain(...); the private handOffToPolling wrapper (which held the duplicated block) is removed.
ReattachCoordinator: the tail of runReattach now calls PollingFallback.drain(...) instead of its own inlined copy.

Why it's safe

Pure internal refactor. The extracted body is character-for-character the same loop that lived in both sites (same terminal-event set, same finish-once discipline, same catch wrapping); behavior is fully determined by the four resume params (lastEmittedPhase/lastEmittedFraction/hasEmittedQueued/hasEmittedFinalizing) the callers already thread.
No isolation change: the old handOffToPolling was a nonisolated static on an actor; drain is likewise a nonisolated static on the PollingFallback actor, and eventStream is nonisolated.
These were the only two occurrences of the drain-into-continuation loop (verified by grep). The two other .failed(.unknown(underlying: error)) yields in ReattachCoordinator are the initial job-status fetch error handlers — a single yield+finish, not the stream-drain loop — and are intentionally left untouched.

Testing

swift build clean.
Full swift test suite green: 226 tests, 28 suites pass. The behavior-covering suites specifically pass: PollingFallback, Reattach transient-retry, ReattachCoordinator.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/comfy-swift-sdk/pull/41 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 7/11/2026 **Status:** ✅ Merged **Merged:** 7/16/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-2855-pollingfallback-drain-helper` --- ### 📝 Commits (1) - [`4e1e915`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/4e1e915c332bffcad8731b5ba2eed1c7e9fe8c02) refactor(polling): centralize PollingFallback stream-drain into one helper ### 📊 Changes **3 files changed** (+58 additions, -68 deletions) <details> <summary>View changed files</summary> 📝 `Sources/ComfySwiftSDK/Internal/PollingFallback.swift` (+47 -0) 📝 `Sources/ComfySwiftSDK/Internal/ReattachCoordinator.swift` (+7 -24) 📝 `Sources/ComfySwiftSDK/Internal/WebSocketSession.swift` (+4 -44) </details> ### 📄 Description ## ELI-5 When a job is running, the SDK normally streams progress over a WebSocket. If that socket drops, the SDK "falls back" to polling: it repeatedly asks the server for status and re-emits those updates to the caller. Two different places did this same fall-back: the WebSocket-drop handoff and the reattach path (resuming a job after the app was backgrounded). Both places had the **exact same block of code** — build the poller, forward every event, and know which events mean "the job is over" so it can close the stream exactly once. Copy-pasted logic like that tends to drift: fix a bug in one and forget the other. This PR pulls that block into a single shared helper both places call, so the "which events are terminal + close-once" contract lives in one spot. ## What changed - Added `PollingFallback.drain(into:transport:jobId:startTime:clock:lastEmittedPhase:lastEmittedFraction:hasEmittedQueued:hasEmittedFinalizing:)` — a static forwarder that builds a `PollingFallback`, drains its `eventStream` into the passed continuation, and applies the terminal-event contract (yield every event; `finish()` once on the first `.complete`/`.failed`/`.cancelled`; `finish()` on natural stream end; wrap any thrown error as `.failed(.unknown(underlying:))` then finish). - `WebSocketSession`: the transient-error handoff now calls `PollingFallback.drain(...)`; the private `handOffToPolling` wrapper (which held the duplicated block) is removed. - `ReattachCoordinator`: the tail of `runReattach` now calls `PollingFallback.drain(...)` instead of its own inlined copy. ## Why it's safe - Pure internal refactor. The extracted body is character-for-character the same loop that lived in both sites (same terminal-event set, same finish-once discipline, same `catch` wrapping); behavior is fully determined by the four resume params (`lastEmittedPhase`/`lastEmittedFraction`/`hasEmittedQueued`/`hasEmittedFinalizing`) the callers already thread. - No isolation change: the old `handOffToPolling` was a nonisolated `static` on an actor; `drain` is likewise a nonisolated `static` on the `PollingFallback` actor, and `eventStream` is `nonisolated`. - These were the **only** two occurrences of the drain-into-continuation loop (verified by grep). The two other `.failed(.unknown(underlying: error))` yields in `ReattachCoordinator` are the *initial job-status fetch* error handlers — a single yield+finish, not the stream-drain loop — and are intentionally left untouched. ## Testing - `swift build` clean. - Full `swift test` suite green: **226 tests, 28 suites** pass. The behavior-covering suites specifically pass: `PollingFallback`, `Reattach transient-retry`, `ReattachCoordinator`. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:34:19 +00:00