Skip to content

Commit

Permalink
frontend: fix websocket logs erros in tests
Browse files Browse the repository at this point in the history
We now mock console logs in tests and do not console errors for tests
env.

Fixes: #2753
Signed-off-by: Kautilya Tripathi <[email protected]>
  • Loading branch information
knrt10 committed Jan 17, 2025
1 parent 59db42b commit 2ac5203
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 37 deletions.
52 changes: 19 additions & 33 deletions frontend/src/lib/k8s/api/v2/webSocket.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import WS from 'vitest-websocket-mock';
import { findKubeconfigByClusterName, getUserIdFromLocalStorage } from '../../../../stateless';
import { getToken } from '../../../auth';
import { getCluster } from '../../../cluster';
import { BASE_WS_URL, useWebSocket, WebSocketManager } from './webSocket';
import { BASE_WS_URL, MULTIPLEXER_ENDPOINT, useWebSocket, WebSocketManager } from './webSocket';

// Mock dependencies
vi.mock('../../../cluster', () => ({
Expand Down Expand Up @@ -37,6 +37,7 @@ describe('WebSocket Tests', () => {
let mockServer: WS;
let onMessage: ReturnType<typeof vi.fn>;
let onError: ReturnType<typeof vi.fn>;
let originalConsoleError: typeof console.error;

beforeEach(() => {
vi.stubEnv('REACT_APP_ENABLE_WEBSOCKET_MULTIPLEXER', 'true');
Expand All @@ -48,10 +49,17 @@ describe('WebSocket Tests', () => {
(getToken as ReturnType<typeof vi.fn>).mockReturnValue(token);
(findKubeconfigByClusterName as ReturnType<typeof vi.fn>).mockResolvedValue({});

mockServer = new WS(`${BASE_WS_URL}wsMultiplexer`);
// Mock console.error for all tests
originalConsoleError = console.error;
console.error = vi.fn();

mockServer = new WS(`${BASE_WS_URL}${MULTIPLEXER_ENDPOINT}`);
});

afterEach(async () => {
// Restore console.error
console.error = originalConsoleError;

WS.clean();
vi.restoreAllMocks();
vi.unstubAllEnvs();
Expand Down Expand Up @@ -211,6 +219,10 @@ describe('WebSocket Tests', () => {
await expect(
WebSocketManager.subscribe(clusterName, '/api/v1/pods', 'watch=true', onMessage)
).rejects.toThrow('WebSocket connection failed');

// Verify error was handled
expect(WebSocketManager.socketMultiplexer).toBeNull();
expect(WebSocketManager.connecting).toBe(false);
});

it('should handle duplicate subscriptions', async () => {
Expand Down Expand Up @@ -422,7 +434,7 @@ describe('WebSocket Tests', () => {
expect(WebSocketManager.connecting).toBe(false);

// Try to use connection again to trigger reconnect
const newServer = new WS(`${BASE_WS_URL}wsMultiplexer`);
const newServer = new WS(`${BASE_WS_URL}${MULTIPLEXER_ENDPOINT}`);
await WebSocketManager.connect();
await newServer.connected;

Expand Down Expand Up @@ -488,46 +500,20 @@ describe('WebSocket Tests', () => {
consoleSpy.mockRestore();
});

it('should handle invalid message format', async () => {
const path = '/api/v1/pods';
const query = 'watch=true';

await WebSocketManager.subscribe(clusterName, path, query, onMessage);
await mockServer.connected;
await mockServer.nextMessage; // Skip subscription

// Send invalid message
await mockServer.send('invalid json');

expect(onMessage).not.toHaveBeenCalled();
});

it('should handle parse errors in message data', async () => {
const path = '/api/v1/pods';
const query = 'watch=true';
const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {});

await WebSocketManager.subscribe(clusterName, path, query, onMessage);
await WebSocketManager.subscribe(clusterName, '/api/v1/pods', 'watch=true', onMessage);
await mockServer.connected;

// Skip subscription message
await mockServer.nextMessage;

// Send malformed data
await mockServer.send(
JSON.stringify({
clusterId: clusterName,
path,
query,
data: 'invalid{json',
type: 'DATA',
path: '/api/v1/pods',
data: 'invalid json',
})
);

expect(onMessage).not.toHaveBeenCalled();
expect(consoleSpy).toHaveBeenCalledWith('Failed to parse update data:', expect.any(Error));

consoleSpy.mockRestore();
expect(console.error).toHaveBeenCalledWith('Failed to parse update data:', expect.any(Error));
});

it('should handle message callback errors in useWebSocket', async () => {
Expand Down
28 changes: 24 additions & 4 deletions frontend/src/lib/k8s/api/v2/webSocket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ import { makeUrl } from './makeUrl';

// Constants for WebSocket connection
export const BASE_WS_URL = BASE_HTTP_URL.replace('http', 'ws');
export const MULTIPLEXER_ENDPOINT = 'wsMultiplexer';

/**
* Multiplexer endpoint for WebSocket connections
* This endpoint allows multiple subscriptions over a single connection
*/
const MULTIPLEXER_ENDPOINT = 'wsMultiplexer';

/**
* Message format for WebSocket communication between client and server.
Expand Down Expand Up @@ -148,8 +148,8 @@ export const WebSocketManager = {
socket.onmessage = this.handleWebSocketMessage.bind(this);

socket.onerror = event => {
console.error('WebSocket error:', event);
this.connecting = false;
console.error('WebSocket error:', event);
reject(new Error('WebSocket connection failed'));
};

Expand Down Expand Up @@ -297,10 +297,23 @@ export const WebSocketManager = {
this.connecting = false;
this.completedPaths.clear();

// Set reconnecting flag if we have active subscriptions
// Only log reconnecting if we have active subscriptions
this.isReconnecting = this.activeSubscriptions.size > 0;
},

/**
* Handles WebSocket error event
* Suppresses expected socket close messages
*/
handleWebSocketError(event: Event): void {
// Only log actual errors, not normal socket closures
if (event instanceof CloseEvent && event.code === 1000 && event.wasClean) {
return;
}
console.error('WebSocket error:', event);
console.error('WebSocket connection failed:', new Error('WebSocket connection failed'));
},

/**
* Handles incoming WebSocket messages
* Processes different message types and notifies appropriate listeners
Expand All @@ -318,6 +331,7 @@ export const WebSocketManager = {
// Handle COMPLETE messages
if (data.type === 'COMPLETE') {
this.completedPaths.add(key);
return;
}

// Parse and validate update data
Expand All @@ -333,7 +347,13 @@ export const WebSocketManager = {
if (update && typeof update === 'object') {
const listeners = this.listeners.get(key);
if (listeners) {
listeners.forEach(listener => listener(update));
for (const listener of listeners) {
try {
listener(update);
} catch (err) {
console.error('Failed to process WebSocket message:', err);
}
}
}
}
} catch (err) {
Expand Down

0 comments on commit 2ac5203

Please sign in to comment.