Skip to content

Commit 9f2a6b4

Browse files
committed
fix(core-backend): improve traces
1 parent 800e615 commit 9f2a6b4

File tree

3 files changed

+217
-148
lines changed

3 files changed

+217
-148
lines changed

packages/core-backend/CHANGELOG.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Changed
11+
12+
- Improve WebSocket connection lifecycle tracing in `BackendWebSocketService` ([#7101](https://github.com/MetaMask/core/pull/7101))
13+
- WebSocket connection duration is now properly reflected in trace span duration instead of only in custom data
14+
- Trace all disconnections (both manual and unexpected) to provide complete connection lifecycle visibility in traces
15+
- Omit `connectionDuration_ms` from disconnection traces when connection never established (onClose without onOpen)
16+
- Update `BackendWebSocketService` to use less aggressive exponential backoff for reconnection
17+
- Increase default `reconnectDelay` from 500 milliseconds to 10 seconds
18+
- Increase default `maxReconnectDelay` from 30 seconds to 60 seconds
19+
20+
### Removed
21+
22+
- Remove `BackendWebSocketService Channel Message` trace as it provided no useful performance insights
23+
1024
## [4.0.0]
1125

1226
### Changed

packages/core-backend/src/BackendWebSocketService.test.ts

Lines changed: 86 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -506,8 +506,8 @@ describe('BackendWebSocketService', () => {
506506
async ({ service }) => {
507507
expect(service.getConnectionInfo().url).toBe('ws://test.example.com');
508508
expect(service.getConnectionInfo().timeout).toBe(10000);
509-
expect(service.getConnectionInfo().reconnectDelay).toBe(500);
510-
expect(service.getConnectionInfo().maxReconnectDelay).toBe(5000);
509+
expect(service.getConnectionInfo().reconnectDelay).toBe(10000);
510+
expect(service.getConnectionInfo().maxReconnectDelay).toBe(60000);
511511
expect(service.getConnectionInfo().requestTimeout).toBe(30000);
512512
expect(service).toBeInstanceOf(BackendWebSocketService);
513513
},
@@ -1087,6 +1087,90 @@ describe('BackendWebSocketService', () => {
10871087
},
10881088
);
10891089
});
1090+
1091+
it('should include connectionDuration_ms in trace when connection was established', async () => {
1092+
const mockTraceFn = jest.fn((_request, fn) => fn?.());
1093+
await withService(
1094+
{
1095+
options: { traceFn: mockTraceFn },
1096+
},
1097+
async ({ service, getMockWebSocket, completeAsyncOperations }) => {
1098+
// Connect and let it establish
1099+
await service.connect();
1100+
await completeAsyncOperations(10);
1101+
1102+
expect(service.getConnectionInfo().state).toBe(
1103+
WebSocketState.CONNECTED,
1104+
);
1105+
1106+
// Clear previous trace calls to focus on disconnection trace
1107+
mockTraceFn.mockClear();
1108+
1109+
// Trigger unexpected close after connection was established
1110+
const mockWs = getMockWebSocket();
1111+
mockWs.simulateClose(1006, 'Abnormal closure');
1112+
await completeAsyncOperations(10);
1113+
1114+
// Find the Disconnection trace call
1115+
const disconnectionTrace = mockTraceFn.mock.calls.find(
1116+
(call) => call[0]?.name === 'BackendWebSocketService Disconnection',
1117+
);
1118+
1119+
expect(disconnectionTrace).toBeDefined();
1120+
expect(disconnectionTrace?.[0]?.data).toHaveProperty(
1121+
'connectionDuration_ms',
1122+
);
1123+
expect(
1124+
disconnectionTrace?.[0]?.data?.connectionDuration_ms,
1125+
).toBeGreaterThan(0);
1126+
},
1127+
);
1128+
});
1129+
1130+
it('should omit connectionDuration_ms in trace when connection never established', async () => {
1131+
const mockTraceFn = jest.fn((_request, fn) => fn?.());
1132+
await withService(
1133+
{
1134+
options: { traceFn: mockTraceFn },
1135+
mockWebSocketOptions: { autoConnect: false },
1136+
},
1137+
async ({ service, getMockWebSocket, completeAsyncOperations }) => {
1138+
// Start connecting
1139+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
1140+
service.connect();
1141+
await completeAsyncOperations(0);
1142+
1143+
expect(service.getConnectionInfo().state).toBe(
1144+
WebSocketState.CONNECTING,
1145+
);
1146+
1147+
// Manually disconnect before connection establishes
1148+
// This will set state to DISCONNECTING and close the WebSocket
1149+
service.disconnect();
1150+
await completeAsyncOperations(0);
1151+
1152+
// Clear trace calls from connection attempt
1153+
mockTraceFn.mockClear();
1154+
1155+
// Simulate the close event after disconnect (state is now DISCONNECTING, not CONNECTING)
1156+
// This will trigger #handleClose with connectedAt = 0
1157+
const mockWs = getMockWebSocket();
1158+
mockWs.simulateClose(1000, 'Normal closure');
1159+
await completeAsyncOperations(10);
1160+
1161+
// Find the Disconnection trace call
1162+
const disconnectionTrace = mockTraceFn.mock.calls.find(
1163+
(call) => call[0]?.name === 'BackendWebSocketService Disconnection',
1164+
);
1165+
1166+
// Trace should exist but should NOT have connectionDuration_ms
1167+
expect(disconnectionTrace).toBeDefined();
1168+
expect(disconnectionTrace?.[0]?.data).not.toHaveProperty(
1169+
'connectionDuration_ms',
1170+
);
1171+
},
1172+
);
1173+
});
10901174
});
10911175

10921176
// =====================================================

0 commit comments

Comments
 (0)