search

digidem / mapeo-core

Library for creating custom geo data and syncronizing via a peer to peer network
23 stars 2 forks source link

this.swarm can be undefined during sync.leave() or sync.join() #89

Closed gmaclennan closed 3 years ago

gmaclennan commented 4 years ago

the sync.join() and sync.leave() methods expect this.swarm to be defined, but this is not always the case. Can they just check if this.swarm exists? Or does sync.leave() need to wait for any in-progress swarm listen-join and then leave?

Reporting because this.swarm is undefined has come up in the mobile app error reports: https://app.bugsnag.com/digital-democracy/mapeo-mobile/errors/5ebac2d0e5d4730017ad4901?i=em&m=ws

hackergrrl commented 4 years ago

Good catch!

Just to document this here: we had a call and discussed having a more explicit JS class or something that managed the swarm state. This could include join triggering a listen and queuing up other join and leave calls.

hackergrrl commented 4 years ago

Thinking more about this. We get to choose our semantics, so if having these methods no-op when there's no swarm is useful, that could be our fix for now?

gmaclennan commented 4 years ago

I think for now, for sync.join() maybe add an optional callback (and pass through to discovery swarm) and call with error if this.swarm is not defined. This way we also can catch any errors in discovery. Then we can push this to a front-end bug to fix: we can expose a failed swarm.join() with a "retry" button or something, but without the callback it will just fail silently without any uncaught error.

For sync.leave() I think a no-op is ok for now.

hackergrrl commented 3 years ago

join now already has logic to queue up the join op until listen gets called, but leave will still crash the process if this.swarm isn't set.

Task: