Refactor the message/queue and syncadaptors

[joshuafontany]

Hi @inmysocks

I have been taking a deep read through Bob's internals recently, and I think there are some race-condition bugs with how you are handling the message-queue/ack system. I keep getting duplicated messages on either the server or client side, and none of the logs would trigger more than once. I traced this back to the $tw.Bob.messageQueueTimer handling, and so I have been reading up on how websockets should troubleshoot these types of connection-issues.

I'd love to chat more directly with you on refactoring the message/queue system and the syncadaptors. I have a very successful "stripped down" version of Bob running on a Dev build of Tiddlywiki (so I could change the signature in methods where the syncer is invoking the syncadaptor).

I have made a lot of progress decoupling the syncadaptor and the message handlers, and on the server using multiple syncer objects (each "attached" to its own wiki) with a single MultiWikiAdaptor object. The saveTiddler flow would then be:

• (Client) User saves a tiddler in the wiki UI -> syncer triggers 'saveTiddler' task in the browser, which calls the BrowserWSAdaptor's saveTiddler method.
• (Client) This sends a ws-message to the connected server.
• The server picks up the message, sends an ack and hands it to the saveTiddler message handler.
• The server message handler simply adds the tiddler to the server-side wiki using native TW methods.
• The server message handler sends back a "Tiddler Saved Serverside"! message as an update on the progress.
• The server side syncer for the affected wiki picks up this change and calls the MultiWikiAdaptor's saveTiddler method.
• The MultiWikiAdaptor sees which wiki the change is coming from, and writes the file to the correct wiki-directory.
• The MultiWikiAdaptor finally sends the client a ws-message saying "Tiddler Saved to Disk!"

At this point, we can then persist the Client/Browser syncer's callback, and trigger it on any of the update messages. I'd prefer the ack or the Tiddler Saved Serverside! (into ram) messages - as that lets the browser's syncer move on to other tasks. And gives us points to handle failure conditions.

Once we separate the logic loop out like this, we can also handle a WS close event, or Heartbeat failures more intelligently. I hope this sparks some ideas.

Best,
Joshua Fontany

[inmysocks]

Thank you for looking at this. I have wanted to rewrite a lot of the logic for the syncer and bobs message queue for a while but I have been focusing on the server components.

What you are suggesting makes a lot of sense.

I don't remember the exact details but the problem I ran into was that Bobs message queue only tries to send one message at a time and doesn't move on until it succeed or fails so it only tries to add one task at a time to the syncer queue and that leads to problems.
The hacks that I put in place for that are probably the source of most of the trouble.

[joshuafontany]

@inmysocks Excellent, I will keep going. Reading through the $tw.syncer and filesystemAdaptor code thoroughly to fix the vanilla node.js TW server's file handling gave me a good idea of where to start. I would love for Bob to use the native $tw.syncer stuff for login/logout, etc tasks. I think it's definitely posible once we solve the task-message-callback logic loop (& I have some good demo code that will help with that).

The other side of it is handling "missing messages" i.e. disconnects. Right now Bob is using the heartbeat Pongs to setup a timer for a Ping. Then a series of timers re-send Pings and test for if($tw.connections[0].socket.readyState !== 1). I have been reading the ws repo on github, and they recommend a different logic flow in their "Pong handler". On a successful "Pong" it destroys a waiting pingTimeout timer, then sets it again to wait the Heartbeat time + a "normal latency expectation". If another Pong doesn't happen in time, it calls terminate() on the websocket.

function heartbeat() {
  clearTimeout(this.pingTimeout);

  // Use `WebSocket#terminate()`, which immediately destroys the connection,
  // instead of `WebSocket#close()`, which waits for the close timer.
  // Delay should be equal to the interval at which your server
  // sends out pings plus a conservative assumption of the latency.
  this.pingTimeout = setTimeout(() => {
    this.terminate();
  }, 30000 + 1000);
}

Then, we can use the ws.onclose handler to attempt reconnects (there is a good method to call them at increasing intervals), and then once reconnected Bob can resend any missed messages back and forth. This will allow an auto-reconnect method, and we only have to show the manual Reconnect button if the largest Reconnect interval fails.

I will let you know when I have a minimum viable demo. Thanks jed!

[tejasvi]

Would love to see the syncing mechanism improved. I am paranoid about using external editor along with Tiddlywiki after losing notes (which thanks to Vim, I was able to salvage). There is a race condition which empties the markdown file when using external editors. With a few tweaks I am able to run Git smoothly however. Will it be possible to integrate PouchDB? I would expect a database designed for syncing to be less buggy and have less maintenance overhead as well.

[inmysocks]

@joshuafontany I believe that recent updates take care of what we discussed here, aside from the websockets reconnecting which I will move to its own issue.

@tejasvi the updates to the syncer and adaptor should make it easier to integrate other storage back-ends but I haven't gotten to the point where I can look into it yet.