Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add automatic retry to stream logs #23098

Merged
merged 1 commit into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions src/data/hassio/supervisor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,25 @@ export const fetchHassioLogsFollow = async (
signal
);

export const fetchHassioLogsFollowSkip = async (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why adding a new command instead of adding an option param to the existing fetchHassioLogsFollow command?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did it because the skip is more complex and needs the cursor and skip lines as addition parameters. But if you want I can rewrite fetchHassioLogsFollow

hass: HomeAssistant,
provider: string,
signal: AbortSignal,
cursor: string,
skipLines: number,
lines = 100,
boot = 0
) =>
hass.callApiRaw(
"GET",
`hassio/${provider.includes("_") ? `addons/${provider}` : provider}/logs${boot !== 0 ? `/boots/${boot}` : ""}/follow`,
undefined,
{
Range: `entries=${cursor}:${skipLines}:${lines}`,
},
signal
);

export const getHassioLogDownloadUrl = (provider: string) =>
`/api/hassio/${
provider.includes("_") ? `addons/${provider}` : provider
Expand Down
90 changes: 61 additions & 29 deletions src/panels/config/logs/error-log-card.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import {
fetchHassioBoots,
fetchHassioLogs,
fetchHassioLogsFollow,
fetchHassioLogsFollowSkip,
fetchHassioLogsLegacy,
getHassioLogDownloadLinesUrl,
getHassioLogDownloadUrl,
Expand Down Expand Up @@ -428,46 +429,66 @@ class ErrorLogCard extends LitElement {
}
}

private async _loadLogs(): Promise<void> {
private async _loadLogs(retry = false): Promise<void> {
this._error = undefined;
this._loadingState = "loading";
this._loadingPrevState = undefined;
this._firstCursor = undefined;
this._numberOfLines = 0;
this._ansiToHtmlElement?.clear();
this._numberOfLines = retry ? (this._numberOfLines ?? 0) : 0;

if (!retry) {
this._loadingPrevState = undefined;
this._firstCursor = undefined;
this._ansiToHtmlElement?.clear();
}

const streamLogs =
this._streamSupported &&
isComponentLoaded(this.hass, "hassio") &&
this.provider;

try {
if (this._logStreamAborter) {
this._logStreamAborter.abort();
this._logStreamAborter = undefined;
}

if (
this._streamSupported &&
isComponentLoaded(this.hass, "hassio") &&
this.provider
) {
if (streamLogs) {
this._logStreamAborter = new AbortController();

// check if there are any logs at all
const testResponse = await fetchHassioLogs(
this.hass,
this.provider,
`entries=:-1:`,
this._boot
);
const testLogs = await testResponse.text();
if (!testLogs.trim()) {
this._loadingState = "empty";
if (!retry) {
// check if there are any logs at all
const testResponse = await fetchHassioLogs(
this.hass,
this.provider!,
`entries=:-1:`,
this._boot
);
const testLogs = await testResponse.text();
if (!testLogs.trim()) {
this._loadingState = "empty";
}
}

const response = await fetchHassioLogsFollow(
this.hass,
this.provider,
this._logStreamAborter.signal,
NUMBER_OF_LINES,
this._boot
);
let response: Response;

if (retry && this._firstCursor) {
response = await fetchHassioLogsFollowSkip(
this.hass,
this.provider!,
this._logStreamAborter.signal,
this._firstCursor,
this._numberOfLines,
NUMBER_OF_LINES,
this._boot
);
} else {
response = await fetchHassioLogsFollow(
this.hass,
this.provider!,
this._logStreamAborter.signal,
NUMBER_OF_LINES,
this._boot
);
}

if (response.headers.has("X-First-Cursor")) {
this._firstCursor = response.headers.get("X-First-Cursor")!;
Expand Down Expand Up @@ -524,14 +545,17 @@ class ErrorLogCard extends LitElement {

if (!this._downloadSupported) {
const downloadUrl = getHassioLogDownloadLinesUrl(
this.provider,
this.provider!,
this._numberOfLines,
this._boot
);
getSignedPath(this.hass, downloadUrl).then((signedUrl) => {
this._logsFileLink = signedUrl.path;
});
}

// first chunk loads successfully, reset retry param
retry = false;
}
}
} else {
Expand All @@ -554,6 +578,13 @@ class ErrorLogCard extends LitElement {
if (err.name === "AbortError") {
return;
}

// The stream can fail if the connection is lost or firefox service worker intercept the connection
if (!retry && streamLogs) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can now also cause an infinite loop I guess? Can we make this check more specific?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There shoulnd't be. Retry is set to false when the first chunk rendered successfully so we say we are in a healty state again. If it fails again beforehand we show the error.

this._loadLogs(true);
return;
}

this._error = (this.localizeFunc || this.hass.localize)(
"ui.panel.config.logs.failed_get_logs",
{
Expand Down Expand Up @@ -590,9 +621,10 @@ class ErrorLogCard extends LitElement {
private _handleConnectionStatus = (ev: HASSDomEvent<ConnectionStatus>) => {
if (ev.detail === "disconnected" && this._logStreamAborter) {
this._logStreamAborter.abort();
this._loadingState = "loading";
}
if (ev.detail === "connected") {
this._loadLogs();
this._loadLogs(true);
}
};

Expand Down
Loading