fix: don't cache shipItLauncher errors across update checks#316
Merged
Conversation
_shipItLauncher wrapped +[SQRLShipItLauncher launchPrivileged:] in replayLazily, which buffers terminal events. If the first subscription errored — the user cancelling the auth prompt (errAuthorizationCanceled) or a transient SMJobSubmit failure — every subsequent check replayed that error without re-invoking launchPrivileged:. Each interval tick would download, verify, write ShipItState.plist, hit the cached error, and delete the staged update, until process restart. Replace replayLazily with a doCompleted: flag so success is still memoized (the launchd job dict is update-agnostic and ShipIt re-reads ShipItState.plist after waitForTermination) but errors fall through and retry on the next subscription. Adds two tests: one locks in the existing submit-once-on-success invariant, the other reproduces the cached-error bug.
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.
Fixes #315.
_shipItLauncherwrapped+[SQRLShipItLauncher launchPrivileged:]inreplayLazily, which buffers terminal events. If the first subscription errored — the user cancelling the auth prompt (errAuthorizationCanceled/-60006) or a transientSMJobSubmitfailure — every subsequent check replayed that error without re-invokinglaunchPrivileged:. Each interval would re-download, re-verify, writeShipItState.plist, hit the cached error, and delete the staged update, until process restart.Replaces
replayLazilywith adoCompleted:flag: success is still memoized (the launchd job dict is update-agnostic; ShipIt re-readsShipItState.plistafterwaitForTermination, so submit-once-per-process was already the contract), but errors fall through and retry on the next subscription.The sole consumer is
then:so the dropped value-replay is a no-op (+launchPrivileged:returns[RACSignal empty]on success anyway).Two new tests:
should not re-submit the launchd job after a successful launch— locks in the existing invariantshould retry the launch on the next subscription after an error— fails onmain, passes here