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

fix(dgw): fallback to recording playback when shadowing is not possible #1181

Merged
18 changes: 17 additions & 1 deletion webapp/player-project/src/main.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { GatewayAccessApi } from './gateway';
import { getPlayer } from './players/index.js';
import { getShadowPlayer } from './streamers/index.js';
import { cleanUpStreamers, getShadowPlayer } from './streamers/index.js';
import './ws-proxy.ts';
import { OnBeforeClose as BeforeWebsocketClose } from './ws-proxy.ts';

async function main() {
const { sessionId, token, gatewayAccessUrl, isActive } = getSessionDetails();
Expand All @@ -23,6 +25,20 @@ async function playSessionShadowing(gatewayAccessApi) {
try {
const recordingInfo = await gatewayAccessApi.fetchRecordingInfo();
const fileType = getFileType(recordingInfo);
BeforeWebsocketClose((closeEvent) => {
if (closeEvent.code !== 1000) {
// faild, try to play static recording
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: This comment is also not clearly worded out. Here is how you could improve that.

Suggested change
// faild, try to play static recording
// The session playback failed; attempt to play the recording as usual as a fallback.

cleanUpStreamers();
playStaticRecording(gatewayAccessApi);
return {
...closeEvent,
// This prevents extra handling by other listeners, particularly for asciinema-player in this scenario.
// For more details, see the asciinema-player WebSocket driver’s socket close handler.
// https://github.com/asciinema/asciinema-player/blob/c09e1d2625450a32e9e76063cdc315fd54ecdd9d/src/driver/websocket.js#L219
code: 1000,
};
}
});

getShadowPlayer(fileType).play(gatewayAccessApi);
} catch (error) {
Expand Down
11 changes: 10 additions & 1 deletion webapp/player-project/src/streamers/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { GatewayAccessApi } from '../gateway';
import { removeTerminal } from '../terminal';
import { handleCast } from './cast';
import { handleWebm } from './webm';

export const getShadowPlayer = (fileType) => {
const player = {
play: (_: GatewayAccessApi) => {},
Expand All @@ -17,3 +17,12 @@ export const getShadowPlayer = (fileType) => {

return player;
};

export const cleanUpStreamers = () => {
// Clean up any existing shadow-player elements
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: This comment is probably unnecessary, but if you want to keep it, here is a better wording.

Suggested change
// Clean up any existing shadow-player elements
// Remove all shadow-player elements.

Copy link
Member

@CBenoit CBenoit Jan 9, 2025

Choose a reason for hiding this comment

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

Note that in this case, it’s much more useful to describe the why, rather than the what. Maybe it makes more sense to explain the why at the callsite of this function.
Here, the why could be something like "Removes the shadow player layout, so the regular recording player is visible again."
The reader of the code now understands the intention, and it’s easier to work from there.

const shadowPlayers = document.querySelectorAll('shadow-player');
for (const shadowPlayer of shadowPlayers) {
shadowPlayer.remove();
}
removeTerminal();
};
7 changes: 7 additions & 0 deletions webapp/player-project/src/terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,10 @@ export function createTerminalDiv() {
document.body.appendChild(terminalDiv);
return terminalDiv;
}

export function removeTerminal() {
const terminalDiv = document.getElementById('terminal');
if (terminalDiv) {
terminalDiv.remove();
}
}
45 changes: 45 additions & 0 deletions webapp/player-project/src/ws-proxy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
let beforeClose = (args: CloseEvent): CloseEvent => {
return args;
};

export const OnBeforeClose = (callback: (args: CloseEvent) => CloseEvent) => {
beforeClose = callback;
};

const WebSocketProxy = new Proxy(window.WebSocket, {
construct(target, args: [url: string | URL, protocols?: string | string[]]) {
console.log('Proxying WebSocket connection', ...args);
const ws = new target(...args); // Create the actual WebSocket instance

// Proxy for intercepting `addEventListener`
ws.addEventListener = new Proxy(ws.addEventListener, {
apply(target, thisArg, args) {
if (args[0] === 'close') {
console.log('Intercepted addEventListener for close event');
const transformedArgs = beforeClose(args as unknown as CloseEvent);
return target.apply(thisArg, transformedArgs);
}
return target.apply(thisArg, args);
},
});

// Proxy for intercepting `onclose`
return new Proxy(ws, {
set(target, prop, value) {
if (prop === 'onclose') {
console.log('Intercepted setting of onclose');
const transformedValue = (...args) => {
const transformedArgs = beforeClose(args as unknown as CloseEvent);
if (typeof value === 'function') {
value(transformedArgs); // Call the original handler
}
};
return Reflect.set(target, prop, transformedValue);
}
return Reflect.set(target, prop, value);
},
});
},
});

window.WebSocket = WebSocketProxy;
Loading