From d42bd36a3ea40c235a59c2b97fe53b9a1f9983f8 Mon Sep 17 00:00:00 2001
From: Simon Lamon <32477463+silamon@users.noreply.github.com>
Date: Thu, 13 Mar 2025 17:18:24 +0100
Subject: [PATCH] Fix logbook keeps loading (#24351)

* Logbook loading

* Redo typing

* small fixes

* async/sync fixes
---
 src/data/logbook.ts              |  4 +-
 src/panels/logbook/ha-logbook.ts | 78 +++++++++++++-------------------
 2 files changed, 33 insertions(+), 49 deletions(-)

diff --git a/src/data/logbook.ts b/src/data/logbook.ts
index ed2c8c2658..d57e27fd26 100644
--- a/src/data/logbook.ts
+++ b/src/data/logbook.ts
@@ -1,4 +1,4 @@
-import type { HassEntity } from "home-assistant-js-websocket";
+import type { HassEntity, UnsubscribeFunc } from "home-assistant-js-websocket";
 import {
   BINARY_STATE_OFF,
   BINARY_STATE_ON,
@@ -119,7 +119,7 @@ export const subscribeLogbook = (
   endDate: string,
   entityIds?: string[],
   deviceIds?: string[]
-): Promise<() => Promise<void>> => {
+): Promise<UnsubscribeFunc> => {
   // If all specified filters are empty lists, we can return an empty list.
   if (
     (entityIds || deviceIds) &&
diff --git a/src/panels/logbook/ha-logbook.ts b/src/panels/logbook/ha-logbook.ts
index b39f10652b..2f404ed0aa 100644
--- a/src/panels/logbook/ha-logbook.ts
+++ b/src/panels/logbook/ha-logbook.ts
@@ -1,6 +1,7 @@
 import type { PropertyValues } from "lit";
 import { css, html, LitElement, nothing } from "lit";
 import { customElement, property, state } from "lit/decorators";
+import type { UnsubscribeFunc } from "home-assistant-js-websocket";
 import { isComponentLoaded } from "../../common/config/is_component_loaded";
 import { computeStateDomain } from "../../common/entity/compute_state_domain";
 import { throttle } from "../../common/util/throttle";
@@ -76,7 +77,7 @@ export class HaLogbook extends LitElement {
 
   @state() private _error?: string;
 
-  private _subscribed?: (() => Promise<void>) | undefined;
+  private _unsubLogbook?: Promise<UnsubscribeFunc>;
 
   private _liveUpdatesEnabled = true;
 
@@ -132,14 +133,14 @@ export class HaLogbook extends LitElement {
   }
 
   public async refresh(force = false) {
-    if (!force && (this._subscribed || this._logbookEntries === undefined)) {
+    if (!force && (this._unsubLogbook || this._logbookEntries === undefined)) {
       return;
     }
 
     this._throttleGetLogbookEntries.cancel();
     this._updateTraceContexts.cancel();
     this._updateUsers.cancel();
-    await this._unsubscribeSetLoading();
+    this._unsubscribe(true);
 
     this._liveUpdatesEnabled = true;
 
@@ -208,16 +209,25 @@ export class HaLogbook extends LitElement {
     );
   }
 
-  private async _unsubscribe(): Promise<void> {
-    if (this._subscribed) {
-      try {
-        await this._subscribed();
-        this._subscribed = undefined;
-        this._pendingStreamMessages = [];
-      } catch (err: any) {
-        // eslint-disable-next-line
-        console.error("Error unsubscribing:", err);
-      }
+  /**
+   * Unsubscribe from a logbook stream since
+   * - we are unloading the page
+   * - we are about to resubscribe
+   * - the entity is not being tracked in the logbook
+   *   and will not return results ever
+   * - the requested start time is in the future
+   *
+   * In cases where no events are expected, we set this._logbookEntries
+   * to an empty list to show a no results message.
+   *
+   * @param loading Indicates if the page should be put in a loading state again.
+   */
+  private _unsubscribe(loading: boolean): void {
+    if (this._unsubLogbook) {
+      this._unsubLogbook.then((unsub) => unsub());
+      this._unsubLogbook = undefined;
+      this._logbookEntries = loading ? undefined : [];
+      this._pendingStreamMessages = [];
     }
   }
 
@@ -231,28 +241,7 @@ export class HaLogbook extends LitElement {
 
   public disconnectedCallback() {
     super.disconnectedCallback();
-    this._unsubscribeSetLoading();
-  }
-
-  /** Unsubscribe because we are unloading
-   * or about to resubscribe.
-   * Setting this._logbookEntries to undefined
-   * will put the page in a loading state.
-   */
-  private async _unsubscribeSetLoading() {
-    await this._unsubscribe();
-    this._logbookEntries = undefined;
-    this._pendingStreamMessages = [];
-  }
-
-  /** Unsubscribe because there are no results.
-   * Setting this._logbookEntries to an empty
-   * list will show a no results message.
-   */
-  private async _unsubscribeNoResults() {
-    await this._unsubscribe();
-    this._logbookEntries = [];
-    this._pendingStreamMessages = [];
+    this._unsubscribe(true);
   }
 
   private _calculateLogbookPeriod() {
@@ -284,20 +273,14 @@ export class HaLogbook extends LitElement {
   private async _subscribeLogbookPeriod(
     logbookPeriod: LogbookTimePeriod
   ): Promise<void> {
-    if (this._subscribed) {
+    if (this._unsubLogbook) {
       return;
     }
+
     try {
-      this._subscribed = await subscribeLogbook(
+      this._unsubLogbook = subscribeLogbook(
         this.hass,
         (streamMessage) => {
-          // "recent" means start time is a sliding window
-          // so we need to calculate an expireTime to
-          // purge old events
-          if (!this._subscribed) {
-            // Message came in before we had a chance to unload
-            return;
-          }
           this._processOrQueueStreamMessage(streamMessage);
         },
         logbookPeriod.startTime.toISOString(),
@@ -305,8 +288,9 @@ export class HaLogbook extends LitElement {
         this.entityIds,
         this.deviceIds
       );
+      await this._unsubLogbook;
     } catch (err: any) {
-      this._subscribed = undefined;
+      this._unsubLogbook = undefined;
       this._error = err;
     }
   }
@@ -315,7 +299,7 @@ export class HaLogbook extends LitElement {
     this._error = undefined;
 
     if (this._filterAlwaysEmptyResults) {
-      await this._unsubscribeNoResults();
+      this._unsubscribe(false);
       return;
     }
 
@@ -323,7 +307,7 @@ export class HaLogbook extends LitElement {
 
     if (logbookPeriod.startTime > logbookPeriod.now) {
       // Time Travel not yet invented
-      await this._unsubscribeNoResults();
+      this._unsubscribe(false);
       return;
     }