[PR #6] [CLOSED] feat(logging): add credential-free os.Logger error logging at SDK catch sites #11

New issue

Closed

opened 2026-10-26 22:33:53 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:33:53 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-swift-sdk/pull/6
Author: @mattmillerai
Created: 6/19/2026
Status: ❌ Closed

Base: main ← Head: matt/be-1608-comfyswiftsdk-add-minimal-error-logging-at-the

📝 Commits (5)

39c1eae fix(transport): treat POSIX socket-drop errors as transient so jobs recover via polling
d94aaeb fix(transport): retry transient errors on the execution_success output fetch
ea35614 merge: main into BE-1606 branch to drop redundant BE-1594 base (it squash-merged)
d765c2a feat(logging): add credential-free os.Logger error logging at SDK catch sites
99caf98 Merge main: resolve WebSocketSession output-build catch (keep logging atop #3's retry-exhausted surface)

📊 Changes

5 files changed (+521 additions, -15 deletions)

View changed files

📝 Sources/ComfySwiftSDK/Internal/PollingFallback.swift (+12 -4)
➕ Sources/ComfySwiftSDK/Internal/SDKLog.swift (+214 -0)
📝 Sources/ComfySwiftSDK/Internal/Transport.swift (+14 -8)
📝 Sources/ComfySwiftSDK/Internal/WebSocketSession.swift (+11 -3)
➕ Tests/ComfySwiftSDKTests/LoggingPrivacyTests.swift (+270 -0)

📄 Description

ELI5

The SDK logged nothing, so when something failed on-device the app only saw the final error with no clue which internal step threw it or why. This adds error-level Console logs at the SDK's failure points — strictly error classification (the ComfyError case, the POSIX code, the jobId), never the API key or token — so the next "background → connection error" repro is self-diagnosing instead of a guess.

What

New Internal/SDKLog.swift: a minimal os.Logger facade (no third-party dependency), subsystem org.comfy.ComfySwiftSDK, categories transport / websocket / polling, with a test-only injectable sink for the privacy test.
.error logs at the Transport.translate, WebSocketSession (output-build / execution_error / read-loop transient-vs-.failed decision), and PollingFallback (give-up / output-assembly / empty-output-exhausted) sites.

Why

First scoped step of the deferred SDK observability — traceability, not full observability. Companion to the background-recovery fixes, so the next on-device failure records the exact catch + POSIX code.

Credential-free (the hard constraint)

Logs only the ComfyError case name, POSIX code, HTTP status, and jobId. Never the token / API key / Authorization / X-API-Key value / request body / response body / raw Error or URLRequest. privacy: .public is applied only to pre-assembled safe fields. The prior NFR-S2 "no logging" guard is re-aimed to "credential-free logging," and a new LoggingPrivacyTests suite drives the failure paths under a captured-log sink and asserts a sentinel key + bearer token never appear (including a poisoned-userInfo case where the sentinel is buried in an NSError description).

Testing

swift build clean; swift test 132 tests, 0 failures (4 live-network skips unchanged). Scope held: error-level only, no remote sink, no new dependency.

_{🔄 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/6 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 6/19/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `matt/be-1608-comfyswiftsdk-add-minimal-error-logging-at-the` --- ### 📝 Commits (5) - [`39c1eae`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/39c1eae04d176c729bcb5b553b3e006b4acb77dd) fix(transport): treat POSIX socket-drop errors as transient so jobs recover via polling - [`d94aaeb`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/d94aaeb62be03a5dc9d4a592adc0a9fd85247f15) fix(transport): retry transient errors on the execution_success output fetch - [`ea35614`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/ea35614f722ef4a4ac8c398c0949cd49bec0a25f) merge: main into BE-1606 branch to drop redundant BE-1594 base (it squash-merged) - [`d765c2a`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/d765c2a81131747c696b4dd2503e7bd407d7850c) feat(logging): add credential-free os.Logger error logging at SDK catch sites - [`99caf98`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/99caf98e232ceac04e46c2433e9ec34e835890b1) Merge main: resolve WebSocketSession output-build catch (keep logging atop #3's retry-exhausted surface) ### 📊 Changes **5 files changed** (+521 additions, -15 deletions) <details> <summary>View changed files</summary> 📝 `Sources/ComfySwiftSDK/Internal/PollingFallback.swift` (+12 -4) ➕ `Sources/ComfySwiftSDK/Internal/SDKLog.swift` (+214 -0) 📝 `Sources/ComfySwiftSDK/Internal/Transport.swift` (+14 -8) 📝 `Sources/ComfySwiftSDK/Internal/WebSocketSession.swift` (+11 -3) ➕ `Tests/ComfySwiftSDKTests/LoggingPrivacyTests.swift` (+270 -0) </details> ### 📄 Description ## ELI5 The SDK logged nothing, so when something failed on-device the app only saw the final error with no clue which internal step threw it or why. This adds error-level Console logs at the SDK's failure points — strictly error *classification* (the `ComfyError` case, the POSIX code, the `jobId`), never the API key or token — so the next "background → connection error" repro is self-diagnosing instead of a guess. ## What - New `Internal/SDKLog.swift`: a minimal `os.Logger` facade (no third-party dependency), subsystem `org.comfy.ComfySwiftSDK`, categories `transport` / `websocket` / `polling`, with a test-only injectable sink for the privacy test. - `.error` logs at the `Transport.translate`, `WebSocketSession` (output-build / execution_error / read-loop transient-vs-`.failed` decision), and `PollingFallback` (give-up / output-assembly / empty-output-exhausted) sites. ## Why First scoped step of the deferred SDK observability — traceability, not full observability. Companion to the background-recovery fixes, so the next on-device failure records the exact catch + POSIX code. ## Credential-free (the hard constraint) Logs only the `ComfyError` case name, POSIX code, HTTP status, and `jobId`. **Never** the token / API key / `Authorization` / `X-API-Key` value / request body / response body / raw `Error` or `URLRequest`. `privacy: .public` is applied only to pre-assembled safe fields. The prior NFR-S2 "no logging" guard is re-aimed to "credential-free logging," and a new `LoggingPrivacyTests` suite drives the failure paths under a captured-log sink and asserts a sentinel key + bearer token never appear (including a poisoned-`userInfo` case where the sentinel is buried in an `NSError` description). ## Testing `swift build` clean; `swift test` 132 tests, 0 failures (4 live-network skips unchanged). Scope held: error-level only, no remote sink, no new dependency. --- <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:53 +00:00