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

resolves websocket errors in tests #2525

Merged
merged 14 commits into from
Feb 17, 2025
22 changes: 19 additions & 3 deletions src/components/TaskAnalysisTable/TaskAnalysisTable.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,13 @@

debouncedUpdateTasks = _debounce(this.updateTasks, 100);

componentWillUnmount() {
// Cancel any pending debounced calls
if (this.debouncedUpdateTasks) {
this.debouncedUpdateTasks.cancel();
}
}

updateTasks(tableState) {
const sortCriteria = {
sortBy: tableState.sorted[0].id,
Expand All @@ -128,7 +135,9 @@

// Use pick instead of cloneDeep, as cloning the entire tableState seems to cause an error
// when any column with a "makeInvertable" header is present.
this.setState({ lastTableState: _pick(tableState, ["sorted", "filtered", "page"]) });
this.setState({
lastTableState: _pick(tableState, ["sorted", "filtered", "page"]),
});

Check warning on line 140 in src/components/TaskAnalysisTable/TaskAnalysisTable.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/TaskAnalysisTable/TaskAnalysisTable.jsx#L138-L140

Added lines #L138 - L140 were not covered by tests
}

configureColumns() {
Expand Down Expand Up @@ -228,7 +237,10 @@
}

if (this.props.criteria?.filters) {
defaultFiltered = _map(this.props.criteria.filters, (value, key) => ({ id: key, value }));
defaultFiltered = _map(this.props.criteria.filters, (value, key) => ({
id: key,
value,
}));

Check warning on line 243 in src/components/TaskAnalysisTable/TaskAnalysisTable.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/TaskAnalysisTable/TaskAnalysisTable.jsx#L240-L243

Added lines #L240 - L243 were not covered by tests
}

const manager = AsManager(this.props.user);
Expand Down Expand Up @@ -725,7 +737,11 @@
{_map(row._original.additionalReviewers, (reviewer, index) => {
return (
<Fragment key={reviewer.username + "-" + index}>
<span style={{ color: AsColoredHashable(reviewer.username).hashColor }}>
<span
style={{
color: AsColoredHashable(reviewer.username).hashColor,
}}
>
{reviewer.username}
</span>
{index + 1 !== row._original.additionalReviewers?.length ? ", " : ""}
Expand Down
12 changes: 7 additions & 5 deletions src/services/Editor/Editor.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ let northEastCorner = null;
let centerPoint = null;
let mapBounds = null;

beforeEach(() => {
beforeAll(() => {
global.WebSocket = class MockWebSocket {
constructor() {
this.onopen = null;
Expand All @@ -35,7 +35,13 @@ beforeEach(() => {
send() {}
close() {}
};
});

afterAll(() => {
delete global.WebSocket;
});

beforeEach(() => {
dispatch = vi.fn();

basicFeature = {
Expand Down Expand Up @@ -134,10 +140,6 @@ beforeEach(() => {
};
});

afterEach(() => {
delete global.WebSocket;
});

describe("osmObjectParams", () => {
beforeEach(() => {
pointFeature.properties.osmid = "123";
Expand Down
85 changes: 77 additions & 8 deletions src/services/Server/WebSocketClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
this.subscriptionHandlers = new Map();
this.serverSubscriptions = new Map();
this.queuedMessages = [];
this.isCleanedUp = false;

this.connect();
}
Expand Down Expand Up @@ -88,19 +89,45 @@
* @private
*/
open() {
if (this.isCleanedUp) {
return;
}

Check warning on line 94 in src/services/Server/WebSocketClient.js

View check run for this annotation

Codecov / codecov/patch

src/services/Server/WebSocketClient.js#L93-L94

Added lines #L93 - L94 were not covered by tests

this.reconnectionHandle = null;
if (this.websocket) {
this.websocket.close();
}

this.websocket = new WebSocket(window.env.REACT_APP_MAP_ROULETTE_SERVER_WEBSOCKET_URL);
this.websocket.onopen = (e) => this.handleOpen(e);
this.websocket.onmessage = (e) => this.handleMessage(e);
this.websocket.onclose = (e) => this.handleClose(e);
// Check if we're in a test environment and use a mock WebSocket if needed
const WebSocketClass =
typeof WebSocket !== "undefined"
? WebSocket
: class MockWebSocket {
constructor() {
this.OPEN = 1;
this.readyState = this.OPEN;
}
close() {}
send() {}

Check warning on line 111 in src/services/Server/WebSocketClient.js

View check run for this annotation

Codecov / codecov/patch

src/services/Server/WebSocketClient.js#L106-L111

Added lines #L106 - L111 were not covered by tests
};

// Use a default URL if window.env is not available (test environment)
const wsUrl =
(typeof window !== "undefined" && window.env?.REACT_APP_MAP_ROULETTE_SERVER_WEBSOCKET_URL) ||
"ws://localhost:9000/ws";

if (!this.pingHandle) {
// Ping the server every 45 seconds to avoid an idle timeout
this.pingHandle = setInterval(() => this.sendPing(), 45000);
try {
this.websocket = new WebSocketClass(wsUrl);
this.websocket.onopen = (e) => !this.isCleanedUp && this.handleOpen(e);
this.websocket.onmessage = (e) => !this.isCleanedUp && this.handleMessage(e);
this.websocket.onclose = (e) => !this.isCleanedUp && this.handleClose(e);

if (!this.pingHandle && !this.isCleanedUp) {
// Ping the server every 45 seconds to avoid an idle timeout
this.pingHandle = setInterval(() => this.sendPing(), 45000);
}
} catch (error) {
console.warn("WebSocket initialization failed:", error);

Check warning on line 130 in src/services/Server/WebSocketClient.js

View check run for this annotation

Codecov / codecov/patch

src/services/Server/WebSocketClient.js#L130

Added line #L130 was not covered by tests
}
}

Expand Down Expand Up @@ -153,12 +180,54 @@
this.sendMessage(pingMessage, true);
}

/**
* Cleanup websocket connection and intervals
*/
cleanup() {
this.isCleanedUp = true;

if (this.websocket) {
try {
this.websocket.onopen = null;
this.websocket.onmessage = null;
this.websocket.onclose = null;
this.websocket.close();
} catch (error) {
console.warn("WebSocket cleanup error:", error);
}

Check warning on line 197 in src/services/Server/WebSocketClient.js

View check run for this annotation

Codecov / codecov/patch

src/services/Server/WebSocketClient.js#L196-L197

Added lines #L196 - L197 were not covered by tests
this.websocket = null;
}

if (this.pingHandle) {
clearInterval(this.pingHandle);
this.pingHandle = null;
}

if (this.reconnectionHandle) {
clearTimeout(this.reconnectionHandle);
this.reconnectionHandle = null;
}

Check warning on line 209 in src/services/Server/WebSocketClient.js

View check run for this annotation

Codecov / codecov/patch

src/services/Server/WebSocketClient.js#L207-L209

Added lines #L207 - L209 were not covered by tests

this.reconnectionAttempts = 0;
this.queuedMessages = [];
}

/**
* Handles websocket close events, attempting to reconnect
*
* @private
*/
handleClose() {
handleClose(e) {

Check warning on line 220 in src/services/Server/WebSocketClient.js

View workflow job for this annotation

GitHub Actions / build (18)

'e' is defined but never used. Allowed unused args must match /^_/u

Check warning on line 220 in src/services/Server/WebSocketClient.js

View workflow job for this annotation

GitHub Actions / build (20)

'e' is defined but never used. Allowed unused args must match /^_/u

Check warning on line 220 in src/services/Server/WebSocketClient.js

View workflow job for this annotation

GitHub Actions / build (18)

'e' is defined but never used. Allowed unused args must match /^_/u

Check warning on line 220 in src/services/Server/WebSocketClient.js

View workflow job for this annotation

GitHub Actions / build (20)

'e' is defined but never used. Allowed unused args must match /^_/u
if (this.isCleanedUp) {
return;
}

Check warning on line 223 in src/services/Server/WebSocketClient.js

View check run for this annotation

Codecov / codecov/patch

src/services/Server/WebSocketClient.js#L222-L223

Added lines #L222 - L223 were not covered by tests

// Clear ping interval on close
if (this.pingHandle) {
clearInterval(this.pingHandle);
this.pingHandle = null;
}

this.connect();
}

Expand Down
Loading