State machine for peers

[gmaclennan]

I've been reading through the current sync state implementation trying to track down a bug (found it was from mis-understanding the events, and also from using kappa v4 on mobile and v3 on desktop). However, reading through I've been thinking about how we can manage this state in a way that:

1. Makes the code more testable
2. Makes is easier to debug
3. Reduces the chances of bugs

Currently the code has several event emitters being passed around inside state (the peer.sync objects) with events being subscribed and unsubscribed at different times. The exact order of these is hard, as is tracking them all. Bearing this in mind and goals above, I was wondering whether the "reducer model" used in a lot of front-end code would be helpful, combined with an API like in #52.

const peerReducer = (peers = new Map(), action) => {
  const updatedPeers = new Map(peers)
  switch (action.type) {
    case 'PEER_DISCOVERED': {
      const id = action.payload
      updatedPeers.set(peer.id, {
        id,
        lastSeen: Date.now()
      })
    }
    case 'PEER_DISCONNECTED': {
      const id = action.payload
      updatedPeers.delete(id)
    }
    case 'START_SYNC': {
      const id = action.payload
      const peer = peers.get(id)
      const { dbHaves, dbWants, mediaHaves, mediaWants } = peer
      updatedPeers.set(id, {
        ...peer,
        status: 'syncing',
        lastSeen: Date.now(),
        syncStart: {
          time: Date.now(),
          dbHaves,
          dbWants,
          mediaHaves,
          mediaWants
        }
      })
    }
    case 'PROGRESS': {
      const { id, dbReceived, dbSent, mediaReceived, mediaSent } = action.payload
      const peer = peers.get(id)
      updatedPeers.set(id, {
        ...peer,
        status: 'syncing',
        lastSeen: Date.now(),
        dbHaves: peer.dbHaves - dbSent,
        dbWants: peer.dbWants - dbReceived
        // etc.
      })
    }
    case 'HANDSHAKE': {
      const { id, dbHaves, dbWants, mediaHaves, mediaWants } = action.payload
      const peer = peers.get(id)
      updatedPeers.set(id, {
        ...peer,
        status: 'ready',
        lastSeen: Date.now(),
        dbHaves,
        dbWants,
        mediaHaves,
        mediaWants
      })
    }
    // etc.
    // etc...
  }
  return updatedPeers
}

One advantage of this pattern is that state is only updated from a single place, and state changes are immutable, so it is very easy to test all the state mutations. We need to think through all different actions that could mutate the state like "peer discovered", "peer handshake" etc. Then we have a single place where we dispatch actions and mutate state e.g.

function createDispatch (reducer, onChange) {
  let state
  return function dispatch (action) {
    const newState = reducer(state, action)
    if (newState !== state) onChange(state)
    state = newState
  }
}

const dispatch = createDispatch(
  peerReducer,
  peers => emitter.emit('peer-update', peers)
)

swarm.on('peer', peer => dispatch({ type: 'PEER_DISCOVERED', payload: peer.id }))
// etc.

Although this code is potentially more boilerplate, it is hopefully easier to reason about and data only flows in one direction, and we hopefully can reduce all the subscriptions and events that we are using, which can get a bit tangled.

[okdistribute]

We definitely need to consolidate & refactor the state code because it currently exists in both mapeo-core and mapeo-mobile/desktop, and the frontends aren't unit tested and minimally e2e tested. Pushing some of this into a sharable module will make it easier to do automatic testing.

[hackergrrl]

Another messy piece to consider is the way that the peer instance and its event emitter are used both interally to mapeo-core and exported externally. This means we can't change any of the internal API of the peer objects or its emitter without needing to do a major semver of the whole module.

Ideally a peer object would have a member that's the publicly-exported version of itself, like

class PeerObject {
  constructor () {
    this.internalEvents = new EventEmitter()
    this.publicEvents = new EventEmitter()
    this.publicPeer = { id: ..., host: ..., port: ..., sync: this.publicEvents }
  }
}

...

peers () {
  return this.peers.map(peer => peer.publicPeer)
}

[gmaclennan]

I don't think we actually need peer objects to be event emitters, or to export them individually. I think the only piece we need in order to render what the user needs to see is an event emitter that will report a list of peer states (where peer=device, not connection).

I think it will be useful to sketch out some ideas about what the API for mapeo.core should look like, and have some discussion about that. A major semver of mapeo-core should not be too difficult to roll out - much easier than changes to the sync protocol.