[PR #5] [MERGED] fix(reattach): retry transient catch-up GET failures instead of failing the stream #10

Closed
opened 2026-10-26 22:33:51 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-swift-sdk/pull/5
Author: @mattmillerai
Created: 6/19/2026
Status: ✅ Merged
Merged: 6/19/2026
Merged by: @mattmillerai

Base: main ← Head: matt/be-1606-reattach-catchup-transient-retry


📝 Commits (1)

  • a7aeb9d fix(reattach): retry transient catch-up GET failures instead of failing the stream

📊 Changes

2 files changed (+154 additions, -4 deletions)

View changed files

📝 Sources/ComfySwiftSDK/Internal/ReattachCoordinator.swift (+40 -4)
➕ Tests/ComfySwiftSDKTests/ReattachTransientRetryTests.swift (+114 -0)

📄 Description

What

When the app foregrounds after a longer background window, ReattachCoordinator's catch-up GET (fetchJobStatus) is the first request on a socket the OS just resumed from suspension, and it frequently fails transiently. The previous code surfaced any catch-up failure as a terminal .failed, falsely killing a job that's usually still recoverable — and for a job that completed while backgrounded, often already done.

Observed on device: ComfyError.network(underlying: NSURLErrorDomain Code=-1011) (NSURLErrorBadServerResponse) during the reconnecting phase → terminal .failed. The error was already correctly classified as a transient .network; the reattach catch-up path just didn't treat it as retryable.

Fix

Wrap the catch-up fetch in a small bounded retry (3 attempts) that retries only PollingFallback.isTransient errors (.network / .offline / .timeout). Non-transient failures — and the final-attempt transient — still propagate, so the stream terminates rather than looping forever.

Tests

  • transient catch-up failure (URL error -1011) → retried → recovers to .complete
  • non-transient failure → fails fast, exactly one attempt, no retry
  • existing reattach tests (incl. auth-failure-on-catch-up → .failed) unchanged

swift build + swift test green (110 tests).

Relationship to #3

#3 hardens the output download path. This PR hardens the reattach catch-up GET path — the one the on-device capture showed failing with -1011 during reconnect. Complementary; both can land.


🔄 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/5 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 6/19/2026 **Status:** ✅ Merged **Merged:** 6/19/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-1606-reattach-catchup-transient-retry` --- ### 📝 Commits (1) - [`a7aeb9d`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/a7aeb9dda75b10daa8d20afbf3b33912b085155b) fix(reattach): retry transient catch-up GET failures instead of failing the stream ### 📊 Changes **2 files changed** (+154 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `Sources/ComfySwiftSDK/Internal/ReattachCoordinator.swift` (+40 -4) ➕ `Tests/ComfySwiftSDKTests/ReattachTransientRetryTests.swift` (+114 -0) </details> ### 📄 Description ## What When the app foregrounds after a longer background window, `ReattachCoordinator`'s catch-up GET (`fetchJobStatus`) is the **first request on a socket the OS just resumed from suspension**, and it frequently fails transiently. The previous code surfaced **any** catch-up failure as a terminal `.failed`, falsely killing a job that's usually still recoverable — and for a job that completed while backgrounded, often already done. Observed on device: `ComfyError.network(underlying: NSURLErrorDomain Code=-1011)` (`NSURLErrorBadServerResponse`) during the `reconnecting` phase → terminal `.failed`. The error was already correctly classified as a transient `.network`; the reattach catch-up path just didn't treat it as retryable. ## Fix Wrap the catch-up fetch in a small bounded retry (3 attempts) that retries **only** `PollingFallback.isTransient` errors (`.network` / `.offline` / `.timeout`). Non-transient failures — and the final-attempt transient — still propagate, so the stream terminates rather than looping forever. ## Tests - transient catch-up failure (URL error -1011) → retried → recovers to `.complete` - non-transient failure → fails fast, exactly one attempt, no retry - existing reattach tests (incl. auth-failure-on-catch-up → `.failed`) unchanged `swift build` + `swift test` green (110 tests). ## Relationship to #3 #3 hardens the output **download** path. This PR hardens the reattach **catch-up GET** path — the one the on-device capture showed failing with -1011 during reconnect. Complementary; both can land. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-26 22:33:51 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Comfy-Org/comfy-swift-sdk#10
No description provided.