fix: map XPC outages cleanly and stop hammering a dead daemon#68
Open
lynicis wants to merge 4 commits into
Open
fix: map XPC outages cleanly and stop hammering a dead daemon#68lynicis wants to merge 4 commits into
lynicis wants to merge 4 commits into
Conversation
Map container XPC connection failures to OrchardError.xpcUnavailable, poll after system start until the service is reachable, and skip service loads while the system is stopped. Harden the UI smoke suite so the mock backend activates reliably and sidebar clicks are hittable.
|
Important Review skippedToo many files! This PR contains 1925 files, which is 1625 over the limit of 300. To get a review, narrow the scope: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1925)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe changes normalise container connectivity errors, poll for confirmed system readiness, restrict UI loading to running systems, and improve UI-test mock-backend launch configuration and readiness handling. ChangesRuntime and UI-test flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SystemService
participant XPCDaemon
participant ContentView
SystemService->>XPCDaemon: checkSystemStatus()
XPCDaemon-->>SystemService: return systemStatus
SystemService->>SystemService: retry until .running or timeout
SystemService-->>ContentView: publish status change
ContentView->>ContentView: performInitialLoad()
Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Orchard/Services/SystemService.swift`:
- Around line 110-121: Keep isSystemLoading true while startSystem() performs
the checkSystemStatus polling loop. Move its reset until after the loop’s ready
or failure outcome, ensuring both the error return and successful continuation
clear the loading state only after XPC readiness resolves.
In `@Orchard/Views/Layout/Content.swift`:
- Around line 189-195: Update the systemStatus .running branch in the Content
view’s onChange handler to call startRefreshTimer() in addition to
performInitialLoad(), ensuring periodic refresh resumes after the timer was
stopped.
- Around line 189-195: Update the systemStatus change handler around
performInitialLoad so repeated .running transitions cannot launch overlapping
loads: serialize or reuse the in-flight initial-load task, preserving the status
re-check in performInitialLoad. Also ensure the refresh timer is restarted when
this .running path follows a stop/start cycle, using the existing timer
lifecycle mechanism rather than creating a separate one.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 26951def-7521-4a2f-8599-7482b4eb0474
📒 Files selected for processing (9)
Orchard.xcodeproj/project.pbxprojOrchard/Services/AppServices.swiftOrchard/Services/ContainerBackend.swiftOrchard/Services/SystemService.swiftOrchard/Services/UITestSupport.swiftOrchard/Views/Layout/Content.swiftOrchard/Views/States/NotRunning.swiftOrchardTests/OrchardErrorTests.swiftOrchardUITests/OrchardUITests.swift
💤 Files with no reviewable changes (1)
- Orchard/Views/States/NotRunning.swift
- Defer resetting isSystemLoading until XPC readiness is confirmed to ensure loading indicator persists correctly - Serialize performInitialLoad execution via task tracking to prevent overlapping loads - Ensure refresh timer correctly restarts after system starts - Invalidate prior refresh timer on restart to prevent leaks
Contributor
Author
|
@andrew-waters due to GitHub runner issue, ui-tests workflow just failed. I guess should rerun it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
When the container daemon is down or XPC is flaky, Orchard was surfacing raw connection errors and still polling services on a dead backend.
connection invalidated, helper communication failures, etc.) toOrchardError.xpcUnavailablecontainer system start, poll untilpingsucceeds before treating the system as runningRelated issues
N/A
Screenshots / recording
No visual redesign — behaviour/error-path and test-harness changes only.
Checklist
Orchardscheme)OrchardTests198/198,OrchardUITests3/3)CHANGELOG.md(Added / Changed / Fixed)Test plan
xcodebuild test -scheme Orchard -destination 'platform=macOS'xcodebuild test -scheme OrchardUITests -destination 'platform=macOS' -only-testing:OrchardUITests/OrchardUITests