Fix logbook keeps loading #24351
Fix logbook keeps loading #24351
Conversation
| 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) { | ||
| if (!subscribePromise) { |
There was a problem hiding this comment.
This will never evaluate to true. It looks like it was put there to handle the case where the component is unloading but events keep coming in. I don't think it did it properly though as it would only work after unsubscribe and at that point there shouldn't be any more events coming.
In any case, we should figure out another condition here or remove it if it is not needed.
| @@ -305,6 +306,7 @@ export class HaLogbook extends LitElement { | |||
| this.entityIds, | |||
| this.deviceIds | |||
| ); | |||
| this._subscribed = await subscribePromise; | |||
There was a problem hiding this comment.
_subscribed is typed as promise, and it also awaited in the code, we now no longer save a promise in _subscribed.
Also now _subscribed is set later, so that when _unsubscribe is called, _subscribed might not be set yet and we never unsubscribe.
| @@ -288,13 +289,13 @@ export class HaLogbook extends LitElement { | |||
| return; | |||
| } | |||
| try { | |||
| this._subscribed = await subscribeLogbook( | |||
There was a problem hiding this comment.
Seems like we already awaited the promise, which is wrong.
Proposed change
2 bug fixes for #24212 introduced after #23853. It's a bit the logic how it is handled that makes it hard to not break anything here along the line.
So, some sensors are continuous, and there's a short circuit in the backend that will provide an empty event straight away after subscription. But the event and the subscription are message wise sent together.
First bug appears that the event is pushed through before the subscription is finalized, and the event is being throw away for being too early according to the comment. I've added the promise to a temporary variable to ensure we know that events that come with the subscription are not being handled as too "early". The subscription promise will resolve shortly after.
Second bug appears on unsubscribing (whenever you change entities for example). The unsubscribe has already happened and the subscription is gone. Giving an error and failing to clean the UI state, resulting in the need to hard refresh.
The demo integration provides a "number.temperature_setting", which you can add on the logbook page.
Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed: