Fix multiple races in logbook subscriptions (#12878)

Co-authored-by: Paulus Schoutsen <balloob@gmail.com>

src/data/logbook.ts

@@ -1,4 +1,4 @@
-import { HassEntity, UnsubscribeFunc } from "home-assistant-js-websocket";
+import { HassEntity } from "home-assistant-js-websocket";
 import {
   BINARY_STATE_OFF,
   BINARY_STATE_ON,
@@ -177,7 +177,7 @@ export const subscribeLogbook = (
   endDate: string,
   entityIds?: string[],
   deviceIds?: string[]
-): Promise<UnsubscribeFunc> => {
+): Promise<() => Promise<void>> => {
   // If all specified filters are empty lists, we can return an empty list.
   if (
     (entityIds || deviceIds) &&

src/panels/logbook/ha-logbook.ts

@@ -1,4 +1,3 @@
-import { UnsubscribeFunc } from "home-assistant-js-websocket";
 import { css, html, LitElement, PropertyValues, TemplateResult } from "lit";
 import { customElement, property, state } from "lit/decorators";
 import { isComponentLoaded } from "../../common/config/is_component_loaded";
@@ -79,7 +78,7 @@ export class HaLogbook extends LitElement {
 
   @state() private _error?: string;
 
-  private _subscribed?: Promise<UnsubscribeFunc | void>;
+  private _subscribed?: Promise<(() => Promise<void>) | void>;
 
   private _throttleGetLogbookEntries = throttle(
     () => this._getLogBookData(),
@@ -136,7 +135,7 @@ export class HaLogbook extends LitElement {
       return;
     }
 
-    this._unsubscribe();
+    this._unsubscribeSetLoading();
     this._throttleGetLogbookEntries.cancel();
     this._updateTraceContexts.cancel();
     this._updateUsers.cancel();
@@ -148,13 +147,19 @@ export class HaLogbook extends LitElement {
       );
     }
 
-    this._logbookEntries = undefined;
     this._throttleGetLogbookEntries();
   }
 
-  protected updated(changedProps: PropertyValues): void {
-    super.updated(changedProps);
+  protected shouldUpdate(changedProps: PropertyValues): boolean {
+    if (changedProps.size !== 1 || !changedProps.has("hass")) {
+      return true;
+    }
+    // We only respond to hass changes if the translations changed
+    const oldHass = changedProps.get("hass") as HomeAssistant | undefined;
+    return !oldHass || oldHass.localize !== this.hass.localize;
+  }
 
+  protected updated(changedProps: PropertyValues): void {
     let changed = changedProps.has("time");
 
     for (const key of ["entityIds", "deviceIds"]) {
@@ -194,7 +199,15 @@ export class HaLogbook extends LitElement {
 
   private _unsubscribe(): void {
     if (this._subscribed) {
-      this._subscribed.then((unsub) => (unsub ? unsub() : undefined));
+      this._subscribed.then((unsub) =>
+        unsub
+          ? unsub().catch(() => {
+              // The backend will cancel the subscription if
+              // we subscribe to entities that will all be
+              // filtered away
+            })
+          : undefined
+      );
       this._subscribed = undefined;
     }
   }
@@ -208,12 +221,26 @@ 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 _unsubscribeSetLoading() {
+    this._logbookEntries = undefined;
     this._unsubscribe();
   }
 
-  private _unsubscribeAndEmptyEntries() {
-    this._unsubscribe();
+  /** Unsubscribe because there are no results.
+   * Setting this._logbookEntries to an empty
+   * list will show a no results message.
+   */
+  private _unsubscribeNoResults() {
     this._logbookEntries = [];
+    this._unsubscribe();
   }
 
   private _calculateLogbookPeriod() {
@@ -252,6 +279,10 @@ export class HaLogbook extends LitElement {
         // "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._processStreamMessage(
           streamMessage,
           "recent" in this.time
@@ -264,8 +295,8 @@ export class HaLogbook extends LitElement {
       ensureArray(this.entityIds),
       ensureArray(this.deviceIds)
     ).catch((err) => {
-      this._error = err.message;
       this._subscribed = undefined;
+      this._error = err;
     });
     return true;
   }
@@ -274,7 +305,7 @@ export class HaLogbook extends LitElement {
     this._error = undefined;
 
     if (this._filterAlwaysEmptyResults) {
-      this._unsubscribeAndEmptyEntries();
+      this._unsubscribeNoResults();
       return;
     }
 
@@ -282,7 +313,7 @@ export class HaLogbook extends LitElement {
 
     if (logbookPeriod.startTime > logbookPeriod.now) {
       // Time Travel not yet invented
-      this._unsubscribeAndEmptyEntries();
+      this._unsubscribeNoResults();
       return;
     }