Discourse: fetcher 404s for user actions as null (#1446)

This is an alternative to solve #1440, taking my
review comments from #1443, to narrow the error handling
to just 404s from the server and crash on other errors.

src/plugins/discourse/fetch.js

@@ -90,8 +90,12 @@ export interface Discourse {
 
   /**
    * Retrieves the like actions that were initiated by the target user.
+   * May be 404 on the server, which will return a null here.
    */
-  likesByUser(targetUsername: string, offset: number): Promise<LikeAction[]>;
+  likesByUser(
+    targetUsername: string,
+    offset: number
+  ): Promise<LikeAction[] | null>;
 }
 
 const MAX_API_REQUESTS_PER_MINUTE = 55;
@@ -199,10 +203,18 @@ export class Fetcher implements Discourse {
     return parsePost(json);
   }
 
-  async likesByUser(username: string, offset: number): Promise<LikeAction[]> {
+  async likesByUser(
+    username: string,
+    offset: number
+  ): Promise<LikeAction[] | null> {
     const response = await this._fetch(
       `/user_actions.json?username=${username}&filter=1&offset=${offset}`
     );
+    const {status} = response;
+    if (status === 404) {
+      // The user probably no longer exists. This is expected, see #1440.
+      return null;
+    }
     failIfMissing(response);
     failForNotOk(response);
     const json = await response.json();

src/plugins/discourse/mirror.js

@@ -456,11 +456,14 @@ export class Mirror implements DiscourseData {
     })();
 
     reporter.start("discourse/likes");
-    const addUserLikes = async (user: string) => {
+    for (const user of this.users()) {
       let offset = 0;
       let upToDate = false;
       while (!upToDate) {
         const likeActions = await this._fetcher.likesByUser(user, offset);
+        if (likeActions == null) {
+          break;
+        }
         possiblePageSize = Math.max(likeActions.length, possiblePageSize);
         for (const like of likeActions) {
           if (addLike(like).doneWithUser) {
@@ -473,16 +476,6 @@ export class Mirror implements DiscourseData {
         }
         offset += likeActions.length;
       }
-    };
-    for (const user of this.users()) {
-      try {
-        await addUserLikes(user);
-      } catch (e) {
-        console.warn(
-          `Warning: Encountered error '${e.message}' ` +
-            `while processing likes for ${user}; skipping this user.`
-        );
-      }
     }
     reporter.finish("discourse/likes");
     reporter.finish("discourse");

src/plugins/discourse/mirror.test.js

@@ -467,22 +467,33 @@ describe("plugins/discourse/mirror", () => {
     });
   });
 
-  it("warns if a user's likes are missing", async () => {
+  it("ignores if a user's likes are missing", async () => {
     const {mirror, fetcher, reporter} = example();
     const pid = fetcher.addPost(1, null, "credbot");
-    (fetcher: any).likesByUser = async () => {
+    (fetcher: any).likesByUser = async () => null;
-      throw new Error("404 - Likes not found");
-    };
     await mirror.update(reporter);
     expect(mirror.topics()).toEqual([fetcher._topic(1)]);
     expect(mirror.posts()).toEqual([fetcher._post(pid)]);
     expect(mirror.likes()).toEqual([]);
-    expect(console.warn).toHaveBeenCalledWith(
+  });
-      "Warning: Encountered error '404 - Likes not found' " +
+
-        "while processing likes for credbot; skipping this user."
+  it("inserts other likes if one user's likes are missing", async () => {
-    );
+    const {mirror, fetcher, reporter} = example();
-    expect(console.warn).toHaveBeenCalledTimes(1);
+    const p1 = fetcher.addPost(1, null, "credbot");
-    spyWarn().mockReset();
+    const p2 = fetcher.addPost(1, 1, "otheruser");
+    const l1 = fetcher.addLike("otheruser", 1, 123);
+    const _likesByUser = fetcher.likesByUser.bind(fetcher);
+    (fetcher: any).likesByUser = async (
+      targetUsername: string,
+      offset: number
+    ) => {
+      if (targetUsername == "credbot") return null;
+      return await _likesByUser(targetUsername, offset);
+    };
+    await mirror.update(reporter);
+    expect(mirror.topics()).toEqual([fetcher._topic(1)]);
+    expect(mirror.posts()).toEqual([fetcher._post(p1), fetcher._post(p2)]);
+    expect(mirror.likes()).toEqual([l1]);
   });
 
   it("sends the right tasks to the TaskReporter", async () => {