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

feat: [M3-8722] - Improve weblish retry behavior #11067

Merged
merged 14 commits into from
Oct 23, 2024
Merged
Show file tree
Hide file tree
Changes from 13 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
31 changes: 25 additions & 6 deletions packages/manager/src/components/ErrorState/ErrorState.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
import ErrorOutline from '@mui/icons-material/ErrorOutline';
import { styled, useTheme } from '@mui/material/styles';
import Grid from '@mui/material/Unstable_Grid2';
import { SxProps, Theme, styled, useTheme } from '@mui/material/styles';
import * as React from 'react';

import { Button } from 'src/components/Button/Button';
import { Typography } from 'src/components/Typography';

import { SvgIconProps } from '../SvgIcon';
import type { SvgIconProps } from '../SvgIcon';
import type { SxProps, Theme } from '@mui/material/styles';

export interface ActionButtonProps {
onClick: () => void;
text: string;
}

export interface ErrorStateProps {
/**
Expand All @@ -16,22 +23,22 @@ export interface ErrorStateProps {
* CSS properties to apply to the custom icon.
*/
CustomIconStyles?: React.CSSProperties;
actionButtonProps?: ActionButtonProps;

/**
* Reduces the padding on the root element.
*/
compact?: boolean;
/**
* The error text to display.
*/
errorText: JSX.Element | string;

/**
* Styles applied to the error text
*/
typographySx?: SxProps<Theme>;
}

export const ErrorState = (props: ErrorStateProps) => {
const { CustomIcon, compact, typographySx } = props;
const { CustomIcon, actionButtonProps, compact, typographySx } = props;
const theme = useTheme();

const sxIcon = {
Expand Down Expand Up @@ -72,6 +79,18 @@ export const ErrorState = (props: ErrorStateProps) => {
) : (
<div style={{ textAlign: 'center' }}>{props.errorText}</div>
)}
{actionButtonProps ? (
<div style={{ textAlign: 'center' }}>
<Button
onClick={() => {
actionButtonProps.onClick?.();
}}
title={actionButtonProps.text}
>
{actionButtonProps.text}
</Button>
</div>
) : null}
</Grid>
</ErrorStateRoot>
);
Expand Down
71 changes: 69 additions & 2 deletions packages/manager/src/features/Lish/Lish.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,76 @@ import type { Tab } from 'src/components/Tabs/TabLinkList';

const AUTH_POLLING_INTERVAL = 2000;

export interface RetryLimiterInterface {
reset: () => void;
retryAllowed: () => boolean;
}

export const RetryLimiter = (
maxTries: number,
perTimeWindowMs: number
): RetryLimiterInterface => {
let retryTimes: number[] = [];

return {
reset: (): void => {
retryTimes = [];
},
retryAllowed: (): boolean => {
const now = Date.now();
retryTimes.push(now);
const cutOffTime = now - perTimeWindowMs;
while (retryTimes.length && retryTimes[0] < cutOffTime) {
retryTimes.shift();
}
return retryTimes.length < maxTries;
},
};
};

export interface LishErrorInterface {
formatted: string;
grn: string;
isExpired: boolean;
reason: string;
}

export const ParsePotentialLishErrorString = (
s: null | string
): LishErrorInterface | null => {
if (!s) {
return null;
}

let parsed = null;
try {
parsed = JSON.parse(s);
Copy link
Contributor

Choose a reason for hiding this comment

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

any chance we know what the type/shape of parsed is, instead of using any?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Until we do the checks following the quoted code, we don't know whether the incoming string actually conforms to the expected shape yet. The whole purpose of this function is to check the shape, and then if it is indeed in the expected shape, return the information that is useful to the code handling retries and errors. If we added an explicit type, it would just be used to cast away the "any" type once the shape is confirmed, then the code would immediately extract a few fields, and then discard that type. I don't think the additional overhead would add to the clarity or safety of the code.

} catch {
return null;
}

const grn = typeof parsed?.grn === 'string' ? parsed?.grn : '';
const grnFormatted = grn ? ` (${grn})` : '';

{
const reason = parsed?.reason;
if (parsed?.type === 'error' && typeof reason === 'string') {
const formattedPrefix = reason.indexOf(' ') >= 0 ? '' : 'Error code: ';
return {
formatted: formattedPrefix + reason + grnFormatted,
grn,
isExpired: reason.toLowerCase() === 'your session has expired.',
reason,
};
}
}
return null;
};

const Lish = () => {
const history = useHistory();

const { isLoading: isMakingInitalRequests } = useInitialRequests();
const { isLoading: isMakingInitialRequests } = useInitialRequests();

const { linodeId, type } = useParams<{ linodeId: string; type: string }>();
const id = Number(linodeId);
Expand All @@ -44,7 +110,8 @@ const Lish = () => {
refetch,
} = useLinodeLishQuery(id);

const isLoading = isLinodeLoading || isTokenLoading || isMakingInitalRequests;
const isLoading =
isLinodeLoading || isTokenLoading || isMakingInitialRequests;

React.useEffect(() => {
const interval = setInterval(checkAuthentication, AUTH_POLLING_INTERVAL);
Expand Down
128 changes: 92 additions & 36 deletions packages/manager/src/features/Lish/Weblish.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,17 @@ import * as React from 'react';

import { CircleProgress } from 'src/components/CircleProgress';
import { ErrorState } from 'src/components/ErrorState/ErrorState';
import {
ParsePotentialLishErrorString,
RetryLimiter,
} from 'src/features/Lish/Lish';

import type { Linode } from '@linode/api-v4/lib/linodes';
import type { LinodeLishData } from '@linode/api-v4/lib/linodes';
import type { Linode } from '@linode/api-v4/lib/linodes';
import type {
LishErrorInterface,
RetryLimiterInterface,
} from 'src/features/Lish/Lish';

interface Props extends Pick<LinodeLishData, 'weblish_url' | 'ws_protocols'> {
linode: Linode;
Expand All @@ -16,15 +24,19 @@ interface Props extends Pick<LinodeLishData, 'weblish_url' | 'ws_protocols'> {
interface State {
error: string;
renderingLish: boolean;
setFocus: boolean;
}

export class Weblish extends React.Component<Props, State> {
lastMessage: string = '';
mounted: boolean = false;
socket: WebSocket;
retryLimiter: RetryLimiterInterface = RetryLimiter(3, 60000);
socket: WebSocket | null;

state: State = {
error: '',
renderingLish: true,
setFocus: false,
};
terminal: Terminal;

Expand All @@ -35,16 +47,17 @@ export class Weblish extends React.Component<Props, State> {

componentDidUpdate(prevProps: Props) {
/*
* If we have a new token, refresh the webosocket connection
* If we have a new token, refresh the websocket connection
* and console with the new token
*/
if (
this.props.weblish_url !== prevProps.weblish_url ||
JSON.stringify(this.props.ws_protocols) !==
JSON.stringify(prevProps.ws_protocols)
) {
this.socket.close();
this.terminal.dispose();
this.socket?.close();
this.terminal?.dispose();
this.setState({ renderingLish: false });
this.connect();
}
}
Expand All @@ -56,22 +69,71 @@ export class Weblish extends React.Component<Props, State> {
connect() {
const { weblish_url, ws_protocols } = this.props;

this.socket = new WebSocket(weblish_url, ws_protocols);
/* When this.socket != origSocket, the socket from this connect()
* call has been closed and possibly replaced by a new socket. */
const origSocket = new WebSocket(weblish_url, ws_protocols);
this.socket = origSocket;

this.lastMessage = '';
this.setState({ error: '' });

this.socket.addEventListener('open', () => {
if (!this.mounted) {
return;
}
this.setState({ renderingLish: true }, () => this.renderTerminal());
this.setState({ renderingLish: true }, () =>
this.renderTerminal(origSocket)
);
});

this.socket.addEventListener('close', (evt) => {
/* If this event is not for the currently active socket, just
* ignore it. */
if (this.socket !== origSocket) {
return;
}
this.socket = null;
this.terminal?.dispose();
this.setState({ renderingLish: false });
/* If the control has been unmounted, the cleanup above is
* sufficient. */
if (!this.mounted) {
return;
}

const parsed: LishErrorInterface | null =
ParsePotentialLishErrorString(evt?.reason) ||
ParsePotentialLishErrorString(this.lastMessage);

if (!this.retryLimiter.retryAllowed()) {
this.setState({
error: parsed?.formatted || 'Unexpected WebSocket close',
});
return;
}
if (parsed?.isExpired) {
const { refreshToken } = this.props;
refreshToken();
return;
}
this.connect();
});
}

render() {
const { error } = this.state;

if (error) {
const actionButtonProps = {
onClick: () => {
this.retryLimiter.reset();
this.props.refreshToken();
},
text: 'Retry Connection',
};
return (
<ErrorState
actionButtonProps={actionButtonProps}
errorText={error}
typographySx={(theme) => ({ color: theme.palette.common.white })}
/>
Expand Down Expand Up @@ -102,10 +164,21 @@ export class Weblish extends React.Component<Props, State> {
);
}

renderTerminal() {
const { linode, refreshToken } = this.props;
renderTerminal(origSocket: WebSocket) {
const { linode } = this.props;
const { group, label } = linode;

const socket: WebSocket | null = this.socket;
if (socket === null || socket !== origSocket) {
return;
}

/* The socket might have already started to fail by the time we
* get here. Leave handling for the close handler. */
if (socket.readyState !== socket.OPEN) {
return;
}

this.terminal = new Terminal({
cols: 80,
cursorBlink: true,
Expand All @@ -114,33 +187,25 @@ export class Weblish extends React.Component<Props, State> {
screenReaderMode: true,
});

this.terminal.onData((data: string) => this.socket.send(data));
this.setState({ setFocus: true }, () => this.terminal.focus());

this.terminal.onData((data: string) => socket.send(data));
const terminalDiv = document.getElementById('terminal');
this.terminal.open(terminalDiv as HTMLElement);

this.terminal.writeln('\x1b[32mLinode Lish Console\x1b[m');

this.socket.addEventListener('message', (evt) => {
let data;

socket.addEventListener('message', (evt) => {
/*
* data is either going to be command line strings
* or it's going to look like {type: 'error', reason: 'thing failed'}
* the latter can be JSON parsed and the other cannot
*
* The actual handling of errors will be done in the 'close'
* handler. Allow the error to be rendered in the terminal in
* case it is actually valid session content that is not
* then followed by a 'close' message.
*/
try {
data = JSON.parse(evt.data);
} catch {
data = evt.data;
}

if (
data?.type === 'error' &&
data?.reason?.toLowerCase() === 'your session has expired.'
) {
refreshToken();
return;
}
this.lastMessage = evt.data;

try {
this.terminal.write(evt.data);
Expand All @@ -157,15 +222,6 @@ export class Weblish extends React.Component<Props, State> {
}
});

this.socket.addEventListener('close', () => {
this.terminal.dispose();
if (!this.mounted) {
return;
}

this.setState({ renderingLish: false });
});

const linodeLabel = group ? `${group}/${label}` : label;
window.document.title = `${linodeLabel} - Linode Lish Console`;
}
Expand Down
Loading