digidem / digidem/comapeo-core
fix: waitForSync('full') should resolve even if the device being synced doesn't have sync capability for all namespaces
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 24
- Forks
- 6
- Avg merge
- 1d 40m
- Merged PRs (30d)
- 8
Description
Description
I think the current implementation of waitForSync() will not work if one of the devices does not have sync capability for one or more namespaces. We should at least have a test for this, and if it fails we need a fix, which I think might be:
function isSynced(state, namespaces, peerSyncControllers) {
for (const ns of namespaces) {
let syncablePeers = 0
for (const psc of peerSyncControllers.values()) {
const { peerId } = psc
if (psc.syncCapability[ns] === 'blocked') continue
if (!(peerId in state[ns].remoteStates)) return false
if (state[ns].remoteStates[peerId].status === 'connecting') return false
syncablePeers++
}
if (syncablePeers > 0 && state[ns].dataToSync) return false
}
return true
}
This came up reviewing #410
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at the JavaScript implementation of waitForSync() and its isSynced() helper; trace how peerSyncControllers, syncCapability, and remoteStates are represented. Add a regression test for a device blocked for some namespaces, then run the relevant test suite and confirm waitForSync('full') resolves when all eligible namespaces are synchronized.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- distributed-systems
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100