fix(fronts): fall back to the in-place edit when the server has no replace route - #59
Merged
Merged
Conversation
…place route
Add-to-front and remove-from-front were switched to POST /fronts/{id}/replace
with no fallback. That endpoint is in no released server yet, and Sheaf is
multi-instance: self-hosters upgrade on their own schedule, so a client that
hard-requires a brand-new route breaks two core actions outright against any
instance that has not caught up.
All six call sites now go through one helper that tries replace and, on 404 or
405, falls back to the in-place PATCH those servers have always had. The
fallback loses the per-member history split, which is exactly the behaviour
those servers already had; nothing gets worse than before the change.
Only 404 and 405 fall back. A 409 means the change itself was refused (a
duplicate member set), and retrying it as a PATCH could quietly apply something
the server had just rejected; auth failures likewise stay failures. A 404 is
ambiguous between "no such route" and "no such front", and that is harmless: if
the front is genuinely gone the PATCH fails the same way, costing one extra
request on a path that was already failing.
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.
Release blocker found during 1.4.0 pre-flight. Follow-up to #56.
The problem
#56 switched add-to-front and remove-from-front to
POST /v1/fronts/{id}/replacewith no fallback. That endpoint is in no released server: the latest server tag isv1.3.4and the route merged after it.Sheaf is multi-instance. A phone can be pointed at any instance, and self-hosters upgrade on their own schedule. So shipping as-is would 404 both of those actions - core, everyday ones - against every instance that hadn't caught up, showing an error and doing nothing.
The fix
All six call sites now go through one helper,
replaceFrontMembers, which tries replace and falls back to the in-placePATCHthose servers have always had.The fallback loses the per-member history split that #56 was about. That is precisely the behaviour those servers already had, so nothing is worse than before #56; instances that do have the route get the better semantics.
What falls back, and what doesn't
404 is ambiguous between "no such route" and "no such front". That's harmless: if the front is genuinely gone, the PATCH fails the same way, costing one extra request on a path that was already failing.
Testing
6 unit tests cover: the happy path uses replace and never PATCHes; 404 and 405 fall back; 409 and 401 propagate without a fallback attempt; and the offline queue's boundary timestamp reaches the replace call.
:app:assemblePlayRelease,:wear:assemblePlayRelease,:app:testPlayReleaseUnitTestgreen.Device checklist, ideally against both an upgraded and an older instance: