Patchwork: private tab is missing messages

[mmckegg]

Maybe a paging issue?

Might also affect other views but harder to tell. It seems that most recent messages are there, but then there is a gap and after that is older messages.

cc @ahdinosaur

[christianbundy]

@mmckegg @ahdinosaur

Any tips on reproducing this? I'm on master reading through all of my private messages and I don't think (?!) there's anything missing.

[mmckegg]

I just had this happen with a message in the public tab too!

I think there is a window of messages between pages that is going missing in some cases. It's hard to give a test case because everyone is seeing different messages in their feeds. And you don't know what you are missing until you do! In this case it was one of my own posts.

Let's call it... the algorithm! 🤣

[christianbundy]

Sorry, I'm wondering which algorithm/heuristic you're using to identify this.

For example, are you scrolling through the posts and notice "hey, I remember I posted on Tuesday at 06:00 but I'm looking at posts from 05:00 and 07:00"? My understanding is that threads are sorted based on the last activity (e.g. likes, not just authorship) and so I'm not sure how [as a human] I'd be able to identify a missing post.

[christianbundy]

Reproduced! A few days ago I sent 16 messages to myself where the message body was a single number incrementing on each message (1, 2, 3, ..., 16). Today I scrolled back through my messages and saw this list of threads:

• [1 message]
• [2 messages]
• [11 messages]
• [1 message]
• [2 messages]
• 16
• 15
• 14
• 13
• 12
• 11
• 10
• 9
• 8
• 7
• [4 messages]

It looks like I have 6 messages missing after the first 14 threads (27 messages).

[christianbundy]

I made a test feed and posted 200 private messages in sequence, then used git bisect to search our history to find the bug's origin. It looks like the problem first occurred in eea148725dd55d17c3f5cbcb7c13776ab9549645, which upgraded our npm dependencies and made some changes to package.json. Maybe this came from upgrading Scuttlebot from 12 to 13?

[christianbundy]

Reverted all of the commit except the Scuttlebot upgrade and I can confirm that this was caused by the upgrade from 12.2.4 to 13.0.3. Here's an easy way to make the skips super simple to find:

index 6e71d76f..d149766f 100644
--- a/modules/page/html/render/private.js
+++ b/modules/page/html/render/private.js
@@ -39,7 +39,7 @@ exports.create = function (api) {

     var getStream = api.sbot.pull.resumeStream((sbot, opts) => {
       return sbot.patchwork.privateFeed.roots(opts)
-    }, { limit: 40, reverse: true })
+    }, { limit: 1, reverse: true })

     var view = api.feed.html.rollup(getStream, {
       prepend: [compose],

Sort of interesting, it goes:

• 200
• 185
• 155
• 125
• 95
• 65
• 35
• 5

My guess is that the bug is hiding near a literal 30 used as a magic number somewhere.

---

That guess may have been wrong.

$ grep --include '*.js' -R -E '30(\W)'|  grep -v '\u'

[mmckegg]

@christianbundy a couple of debugging ideas (if you haven't already)

• doing this query directly in sbot and see what happens
• putting some pull.through probes in sbot/private-feed and seeing if the messages get lost here or somewhere else

[christianbundy]

Thanks Matt. I'm not sure how to get the query and do it directly in sbot, and I'm having trouble making a minimal test-case because I can't get depject to play nicely, but I can confirm that the console.log output here is also missing the messages:

diff --git a/modules/sbot.js b/modules/sbot.js
index 61cf61b1..3e257769 100644
--- a/modules/sbot.js
+++ b/modules/sbot.js
@@ -263,17 +263,23 @@ exports.create = function (api) {
         resumeStream: function (fn, baseOpts) {
           return function (opts) {
             var stream = defer.source()
             onceTrue(connection, function (connection) {
               stream.resolve(pullResume.remote((opts) => {
                 return fn(connection, opts)
               }, extend(baseOpts, opts)))
             })
-            return stream
+            return pull(
+              stream,
+              pull.through((msg) => {
+                console.log(msg && msg.value && msg.value.content && msg.value.content.text)
+                return msg
+              })
+            )
           }
         }
       },
       obs: {
         connectionStatus: (listener) => connectionStatus(listener),
         connection,
         connectedPeers: () => connectedPeers,
         localPeers: () => localPeers

I suppose maybe I can get the query with console.log(opts) or something? Also, is there a way to load just modules/sbot.js and its dependencies with sbot? I'm apparently really bad at depject.

[christianbundy]

Confirming that this console.log also has the same problem, so it's further up the stream / stack.

diff --git a/sbot/private-feed.js b/sbot/private-feed.js
index e77db292..2c2f33b4 100644
--- a/sbot/private-feed.js
+++ b/sbot/private-feed.js
@@ -88,6 +88,7 @@ exports.init = function (ssb, config) {

           // ADD THREAD SUMMARY
           Paramap((item, cb) => {
+            console.log(item.value.content)
             threadSummary(item.key, {
               recentLimit: 3,
               readThread: ssb.patchwork.thread.read,

[christianbundy]

I think I've got an understanding for the bug. If I move pull.take() above pull.through() then it solves the problem, but I'm not sure how that would change the limit logic.

diff --git a/lib/pull-resume.js b/lib/pull-resume.js
index 42b17963..28bcf046 100644
--- a/lib/pull-resume.js
+++ b/lib/pull-resume.js
@@ -8,16 +8,34 @@ module.exports = {
     if (limit) {
       let marker = { marker: true }
       let count = 0
+
+      // Assume an alphabet of messages.
       return pullCat([
         pull(
+          // First we hook our hose up to the alphabet faucet.
           stream,
+
+          // Then we record each letter as it comes through.
+          // A B C D E F G H I J K L
+          // Twelve letters have come through, if the stream abruptly ends we
+          // know we'll want to restart the stream past L.
           pull.through(msg => {
             if (!msg.sync) {
               marker.resume = getResume(msg)
             }
           }),
+
+          // Then we filter out the vowels, because vowels are Very Bad.
+          // B C D F G H J K L
           filterMap,
+
+          // We only want six, so we remove the rest.
+          // B C D F G H
           pull.take(limit),
+
+          // Done! But what happened to K and L?
+          // The marker counted them but they weren't taken because of the filter.
+
           pull.through(() => {
             count += 1
           })