re-addressing "429: too many requests"#94
Merged
Conversation
might be rate limited by some reverse proxies
URLSession.dataTaskPublisher emits on a background queue. The mutation sinks in ProjectManager (sendBill, deleteBill, sendMember, deleteMember) forwarded that thread straight into user-supplied completion handlers, which then touched @State, @binding and @published on the caller side producing "Publishing changes from background threads is not allowed" runtime warnings after every save / delete / add.
Contributor
There was a problem hiding this comment.
Pull request overview
Re-addresses the HTTP 429 "Too Many Requests" issue from #81 by eliminating duplicate/burst requests to the Cospend backend and fixing a handful of related SwiftUI state-management bugs that were masking whether the fix worked.
Changes:
- Make
loadBillsAndMemberssingle-flight (cancel-and-replace via a storedAnyCancellable) and add anasync refresh()wrapper used by.refreshable; replace.onAppearreloads inBillList/BalanceListwith pull-to-refresh +scenePhasereload; hoist tab view models to@StateObject. - Strip the leading
/fromProjectBackend.staticPathso a server URL with a trailing slash no longer produces//, and add a regression test. - Fix unrelated SwiftUI bugs:
sortedBillsnow reacts tocurrentProjectviaCombineLatest; convert@State/@Bindinginput properties onBalanceCell,BillCell,WhoPaidView,PersonText,PersonsViewtolet; route background-thread@Publishedwrites throughDispatchQueue.main.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| PayForMe/Model/Project.swift | Removes leading slash from staticPath for both backends to avoid // when server URL has a trailing slash. |
| PayForMe/Services/ProjectManager.swift | Adds single-flight loadCancellable, async refresh() wrapper, and .receive(on: .main) on mutation publishers; cancels in-flight load on project switch. |
| PayForMe/Views/ContentView.swift | Promotes tab view models to @StateObject and reloads on scenePhase == .active. |
| PayForMe/Views/BillList/BillList.swift | Replaces .onAppear reload with .refreshable calling ProjectManager.refresh(). |
| PayForMe/Views/BillList/BillListViewModel.swift | Uses CombineLatest($sortBy, $currentProject) so sortedBills updates when bills change. |
| PayForMe/Views/BillList/BillCell.swift | Switches bill from @State to let; passes plain values to PersonsView. |
| PayForMe/Views/BillList/PersonsView.swift | Switches bill and members from @Binding to let. |
| PayForMe/Views/BillDetail/WhoPaidView.swift | Switches members from @State to let. |
| PayForMe/Views/PersonText.swift | Switches person from @State to let. |
| PayForMe/Views/Balance/BalanceList.swift | Replaces .onAppear reload with .refreshable; BalanceCell.balance becomes let. |
| PayForMe/Views/Projects/QRCodes/AddProjectQRViewModel.swift | Routes passwordCorrect, isTestingSubject, and foundCode through the main queue. |
| PayForMeTests/NetworkRequestTests.swift | Adds regression test verifying a trailing slash in the server URL no longer produces //. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
I could finally reproduce the 429 myself and went digging. The fix from #92 only touched the validate-while-typing path, but the regular bills/members loader had its own request-burst problem: every
onAppear, every mutation, every tab switch fired a fresh Combine pipeline without cancelling the previous one, and a server URL ending in / produced a double slash (https://host//index.php/...) which openresty seems to rate-limit as a separate path.I removed the leading / from staticPath, made
loadBillsAndMemberssingle-flight (cancel-and-replace via oneAnyCancellable), put the tab view models behind@StateObjectso they don't get re-allocated on every render, and replaced the.onAppearreloads on the bills and balance tab with.refreshableplus a refresh on scenePhase becoming active.While I was at it, a few related bugs got in the way of confirming the fix:
sortedBillswasn't reacting to new bills (only watching$sortBy), and four views had@Stateon input properties (BalanceCell, BillCell, WhoPaidView, PersonText) which is why balances and edited bills only updated after reopening the project. Also fixed two background-thread@Publishedwrites that showed up in the console (mutation completion handlers and the QR scan publisher).Until another occurrence, this merge actually closes #81.