fix(sync): better fault tolerance and logging during sync

sleidig · sleidig · commit 68c4aae248c1 · 2024-07-29T18:28:25.000+02:00
diff --git a/src/app/core/database/pouch-database.spec.ts b/src/app/core/database/pouch-database.spec.ts
@@ -16,6 +16,9 @@
  */
 
 import { PouchDatabase } from "./pouch-database";
+import { fakeAsync, tick } from "@angular/core/testing";
+import PouchDB from "pouchdb-browser";
+import { HttpStatusCode } from "@angular/common/http";
 
 describe("PouchDatabase tests", () => {
   let database: PouchDatabase;
@@ -284,4 +287,36 @@ describe("PouchDatabase tests", () => {
 
     await expectAsync(database.isEmpty()).toBeResolvedTo(false);
   });
+
+  it("should try auto-login if fetch fails and fetch again", fakeAsync(() => {
+    const mockAuthService = jasmine.createSpyObj(["login", "addAuthHeader"]);
+    database = new PouchDatabase(mockAuthService);
+    database.initRemoteDB("");
+
+    mockAuthService.login.and.resolveTo();
+    // providing "valid" token on second call
+    let calls = 0;
+    mockAuthService.addAuthHeader.and.callFake((headers) => {
+      headers.Authorization = calls % 2 === 1 ? "valid" : "invalid";
+    });
+    spyOn(PouchDB, "fetch").and.callFake(async (url, opts) => {
+      calls++;
+      if (opts.headers["Authorization"] === "valid") {
+        return new Response('{ "_id": "foo" }', { status: HttpStatusCode.Ok });
+      } else {
+        return {
+          status: HttpStatusCode.Unauthorized,
+          ok: false,
+        } as Response;
+      }
+    });
+
+    database.get("Entity:ABC");
+    tick();
+    tick();
+
+    expect(PouchDB.fetch).toHaveBeenCalledTimes(2);
+    expect(mockAuthService.login).toHaveBeenCalled();
+    expect(mockAuthService.addAuthHeader).toHaveBeenCalledTimes(2);
+  }));
 });
diff --git a/src/app/core/database/pouch-database.ts b/src/app/core/database/pouch-database.ts
@@ -20,11 +20,12 @@ import { Logging } from "../logging/logging.service";
 import PouchDB from "pouchdb-browser";
 import memory from "pouchdb-adapter-memory";
 import { PerformanceAnalysisLogging } from "../../utils/performance-analysis-logging";
-import { Injectable } from "@angular/core";
+import { Injectable, Optional } from "@angular/core";
 import { firstValueFrom, Observable, Subject } from "rxjs";
 import { filter } from "rxjs/operators";
 import { HttpStatusCode } from "@angular/common/http";
 import { environment } from "../../../environments/environment";
+import { KeycloakAuthService } from "../session/auth/keycloak/keycloak-auth.service";
 
 /**
  * Wrapper for a PouchDB instance to decouple the code from
@@ -66,7 +67,7 @@ export class PouchDatabase extends Database {
   /**
    * Create a PouchDB database manager.
    */
-  constructor() {
+  constructor(@Optional() private authService?: KeycloakAuthService) {
     super();
   }
 
@@ -105,27 +106,55 @@ export class PouchDatabase extends Database {
    */
   initRemoteDB(
     dbName = `${environment.DB_PROXY_PREFIX}/${environment.DB_NAME}`,
-    fetch = this.defaultFetch,
   ): PouchDatabase {
     const options = {
       adapter: "http",
       skip_setup: true,
-      fetch,
+      fetch: (url: string | Request, opts: RequestInit) =>
+        this.defaultFetch(url, opts),
     };
     this.pouchDB = new PouchDB(dbName, options);
     this.databaseInitialized.complete();
     return this;
   }
 
-  private defaultFetch(url, opts: any) {
+  private defaultFetch: Fetch = async (url: string | Request, opts: any) => {
     if (typeof url !== "string") {
-      return;
+      const err = new Error("PouchDatabase.fetch: url is not a string");
+      err["details"] = url;
+      throw err;
     }
 
     const remoteUrl =
       environment.DB_PROXY_PREFIX + url.split(environment.DB_PROXY_PREFIX)[1];
-    return PouchDB.fetch(remoteUrl, opts);
-  }
+    this.authService.addAuthHeader(opts.headers);
+
+    let result: Response;
+    try {
+      result = await PouchDB.fetch(remoteUrl, opts);
+    } catch (err) {
+      Logging.warn("Failed to fetch from DB", err);
+    }
+
+    // retry login if request failed with unauthorized
+    if (!result || result.status === HttpStatusCode.Unauthorized) {
+      try {
+        await this.authService.login();
+        this.authService.addAuthHeader(opts.headers);
+        result = await PouchDB.fetch(remoteUrl, opts);
+      } catch (err) {
+        Logging.warn("Failed to fetch from DB", err);
+      }
+    }
+
+    if (!result || result.status >= 500) {
+      throw new DatabaseException({
+        error: "Failed to fetch from DB",
+        actualResponse: result,
+      });
+    }
+    return result;
+  };
 
   async getPouchDBOnceReady(): Promise<PouchDB.Database> {
     await firstValueFrom(this.databaseInitialized, {
@@ -438,7 +467,10 @@ export class PouchDatabase extends Database {
  * This overwrites PouchDB's error class which only logs limited information
  */
 class DatabaseException extends Error {
-  constructor(error: PouchDB.Core.Error, entityId?: string) {
+  constructor(
+    error: PouchDB.Core.Error | { [key: string]: any },
+    entityId?: string,
+  ) {
     super();
 
     if (entityId) {
diff --git a/src/app/core/database/sync.service.spec.ts b/src/app/core/database/sync.service.spec.ts
@@ -6,8 +6,6 @@ import { Database } from "./database";
 import { LoginStateSubject, SyncStateSubject } from "../session/session-type";
 import { LoginState } from "../session/session-states/login-state.enum";
 import { KeycloakAuthService } from "../session/auth/keycloak/keycloak-auth.service";
-import { HttpStatusCode } from "@angular/common/http";
-import PouchDB from "pouchdb-browser";
 import { Subject } from "rxjs";
 
 describe("SyncService", () => {
@@ -72,47 +70,6 @@ describe("SyncService", () => {
     stopPeriodicTimer();
   }));
 
-  it("should try auto-login if fetch fails and fetch again", fakeAsync(() => {
-    // Make sync call pass
-    spyOn(
-      TestBed.inject(Database) as PouchDatabase,
-      "getPouchDB",
-    ).and.returnValues({ sync: () => Promise.resolve() } as any);
-    spyOn(PouchDB, "fetch").and.returnValues(
-      Promise.resolve({
-        status: HttpStatusCode.Unauthorized,
-        ok: false,
-      } as Response),
-      Promise.resolve({ status: HttpStatusCode.Ok, ok: true } as Response),
-    );
-    // providing "valid" token on second call
-    let calls = 0;
-    mockAuthService.addAuthHeader.and.callFake((headers) => {
-      headers.Authorization = calls++ === 1 ? "valid" : "invalid";
-    });
-    mockAuthService.login.and.resolveTo();
-    const initSpy = spyOn(service["remoteDatabase"], "initRemoteDB");
-    service.startSync();
-    tick();
-    // taking fetch function from init call
-    const fetch = initSpy.calls.mostRecent().args[1];
-
-    const url = "/db/_changes";
-    const opts = { headers: {} };
-    let fetchResult;
-    fetch(url, opts).then((res) => (fetchResult = res));
-    tick();
-    expect(fetchResult).toBeDefined();
-
-    expect(PouchDB.fetch).toHaveBeenCalledTimes(2);
-    expect(PouchDB.fetch).toHaveBeenCalledWith(url, opts);
-    expect(opts.headers).toEqual({ Authorization: "valid" });
-    expect(mockAuthService.login).toHaveBeenCalled();
-    expect(mockAuthService.addAuthHeader).toHaveBeenCalledTimes(2);
-
-    stopPeriodicTimer();
-  }));
-
   it("should sync immediately when local db has changes", fakeAsync(() => {
     const mockLocalDb = jasmine.createSpyObj(["sync"]);
     const db = TestBed.inject(Database) as PouchDatabase;
diff --git a/src/app/core/database/sync.service.ts b/src/app/core/database/sync.service.ts
@@ -2,8 +2,6 @@ import { Injectable } from "@angular/core";
 import { Database } from "./database";
 import { PouchDatabase } from "./pouch-database";
 import { Logging } from "../logging/logging.service";
-import { HttpStatusCode } from "@angular/common/http";
-import PouchDB from "pouchdb-browser";
 import { SyncState } from "../session/session-states/sync-state.enum";
 import { SyncStateSubject } from "../session/session-type";
 import {
@@ -30,7 +28,7 @@ export class SyncService {
   private readonly POUCHDB_SYNC_BATCH_SIZE = 500;
   static readonly SYNC_INTERVAL = 30000;
 
-  private remoteDatabase = new PouchDatabase();
+  private remoteDatabase: PouchDatabase;
   private remoteDB: PouchDB.Database;
   private localDB: PouchDB.Database;
 
@@ -39,6 +37,8 @@ export class SyncService {
     private authService: KeycloakAuthService,
     private syncStateSubject: SyncStateSubject,
   ) {
+    this.remoteDatabase = new PouchDatabase(this.authService);
+
     this.logSyncContext();
 
     this.syncStateSubject
@@ -78,45 +78,13 @@ export class SyncService {
   private initDatabases() {
     this.remoteDatabase.initRemoteDB(
       `${environment.DB_PROXY_PREFIX}/${environment.DB_NAME}`,
-      this.fetch.bind(this),
     );
     this.remoteDB = this.remoteDatabase.getPouchDB();
     if (this.database instanceof PouchDatabase) {
       this.localDB = this.database.getPouchDB();
     }
   }
 
-  private async fetch(url: string, opts: any) {
-    // TODO: merge this with PouchDatabase.defaultFetch, which is very similar
-
-    if (typeof url !== "string") {
-      return;
-    }
-
-    const remoteUrl =
-      environment.DB_PROXY_PREFIX + url.split(environment.DB_PROXY_PREFIX)[1];
-    const initialRes = await this.sendRequest(remoteUrl, opts);
-
-    // retry login if request failed with unauthorized
-    if (initialRes.status === HttpStatusCode.Unauthorized) {
-      return (
-        this.authService
-          .login()
-          .then(() => this.sendRequest(remoteUrl, opts))
-          // return initial response if request failed again
-          .then((newRes) => (newRes.ok ? newRes : initialRes))
-          .catch(() => initialRes)
-      );
-    } else {
-      return initialRes;
-    }
-  }
-
-  private sendRequest(url: string, opts) {
-    this.authService.addAuthHeader(opts.headers);
-    return PouchDB.fetch(url, opts);
-  }
-
   /**
    * Execute a (one-time) sync between the local and server database.
    */
