mirror of
https://github.com/jambonz/jambonz-feature-server.git
synced 2026-10-04 02:04:12 +00:00
The ws v8 library throws a plain Error('Opening handshake has timed out') with no
.code and name 'Error' when the handshake timeout (JAMBONES_WS_HANDSHAKE_TIMEOUT_MS,
default 1500ms) elapses. BaseRequestor._shouldRetry classified retryable errors only
by .code/.name/.statusCode, so this error matched neither the ct nor rt bucket and was
never retried under the default rp=ct policy — the call failed immediately on the first
attempt, silently bypassing maxReconnects.
Match the handshake-timeout message in the ct bucket so a transient slow WS upgrade
(e.g. a serverless/edge backend cold start) is retried as intended. Adds tape coverage
for both the default-ct retry path and the rp=4xx no-retry path.
This commit is contained in:
@@ -6,6 +6,11 @@ const timeSeries = require('@jambonz/time-series');
|
||||
const {NODE_ENV, JAMBONES_TIME_SERIES_HOST} = require('../config');
|
||||
let alerter ;
|
||||
|
||||
// The ws (v8) library throws this plain Error on an opening-handshake timeout. It carries no
|
||||
// .code and .name === 'Error', so it must be matched by message to be treated as a connection
|
||||
// timeout (ct) for retry purposes (issue #1565).
|
||||
const WS_HANDSHAKE_TIMEOUT_MESSAGE = 'Opening handshake has timed out';
|
||||
|
||||
class BaseRequestor extends Emitter {
|
||||
constructor(logger, account_sid, hook, secret) {
|
||||
super();
|
||||
@@ -99,11 +104,13 @@ class BaseRequestor extends Emitter {
|
||||
* @returns {boolean} True if the error should be retried
|
||||
*/
|
||||
_shouldRetry(err, rpValues) {
|
||||
// ct = connection timeout (ECONNREFUSED, ETIMEDOUT, etc)
|
||||
// ct = connection timeout (ECONNREFUSED, ETIMEDOUT, etc). The ws opening-handshake timeout
|
||||
// has no .code, so match it by message so the default ct policy retries it (issue #1565).
|
||||
const isCt = err.code === 'ECONNREFUSED' ||
|
||||
err.code === 'ETIMEDOUT' ||
|
||||
err.code === 'ECONNRESET' ||
|
||||
err.code === 'ECONNABORTED';
|
||||
err.code === 'ECONNABORTED' ||
|
||||
err.message === WS_HANDSHAKE_TIMEOUT_MESSAGE;
|
||||
// rt = request timeout
|
||||
const isRt = err.name === 'TimeoutError';
|
||||
// 4xx = client errors
|
||||
|
||||
@@ -8,6 +8,9 @@ const {
|
||||
} = require('../lib/config');
|
||||
const logger = require('pino')({level: JAMBONES_LOGLEVEL});
|
||||
|
||||
// The exact message the ws v8 library throws on an opening-handshake timeout (issue #1565).
|
||||
const HANDSHAKE_TIMEOUT_MESSAGE = 'Opening handshake has timed out';
|
||||
|
||||
// Mock WebSocket specifically for retry testing
|
||||
class RetryMockWebSocket {
|
||||
static retryScenarios = new Map();
|
||||
@@ -124,6 +127,15 @@ class RetryMockWebSocket {
|
||||
this.eventListeners.get('error')(err);
|
||||
}
|
||||
});
|
||||
} else if (behavior.type === 'handshake-timeout') {
|
||||
// Simulate the ws v8 opening-handshake timeout: a plain Error with no
|
||||
// .code property and .name === 'Error' (see issue #1565).
|
||||
setImmediate(() => {
|
||||
console.log(`RetryMockWebSocket: triggering handshake timeout`);
|
||||
if (this.eventListeners.has('error')) {
|
||||
this.eventListeners.get('error')(new Error(HANDSHAKE_TIMEOUT_MESSAGE));
|
||||
}
|
||||
});
|
||||
} else if (behavior.type === 'success') {
|
||||
// Successful connection
|
||||
console.log(`RetryMockWebSocket: triggering success`);
|
||||
@@ -572,6 +584,82 @@ test('WS Retry - rp=ct (connection timeout) should retry network errors', async
|
||||
t.end();
|
||||
});
|
||||
|
||||
test('WS Retry - handshake timeout with default (ct) policy should retry and succeed', async (t) => {
|
||||
// GIVEN a handshake-timeout error (ws v8 throws a code-less Error) on the first
|
||||
// attempt, and the default retry policy (no hash params => ct). Regression for #1565:
|
||||
// this error type was never classified as retryable, so the call failed on attempt 1.
|
||||
RetryMockWebSocket.clearScenarios();
|
||||
|
||||
const retryScenario = {
|
||||
attempts: [
|
||||
{ type: 'handshake-timeout' },
|
||||
{ type: 'success' }
|
||||
]
|
||||
};
|
||||
RetryMockWebSocket.setRetryScenario('ws://localhost:3000', retryScenario);
|
||||
|
||||
const hook = {
|
||||
url: 'ws://localhost:3000', // No hash parameters - defaults to ct policy
|
||||
username: 'username',
|
||||
password: 'password'
|
||||
};
|
||||
|
||||
const params = {
|
||||
callSid: 'test_handshake_timeout_ct'
|
||||
};
|
||||
|
||||
// WHEN
|
||||
const requestor = new WsRequestor(logger, "account_sid", hook, "webhook_secret");
|
||||
const result = await requestor.request('session:new', hook, params, {});
|
||||
|
||||
// THEN
|
||||
t.ok(result, 'ws retried the handshake timeout under the default ct policy and got a response');
|
||||
t.equal(RetryMockWebSocket.getConnectionAttempts('ws://localhost:3000'), 2,
|
||||
'should have made 2 connection attempts');
|
||||
t.end();
|
||||
});
|
||||
|
||||
test('WS Retry - handshake timeout with rp=4xx should not retry', async (t) => {
|
||||
// GIVEN a handshake-timeout error but a retry policy that only covers 4xx.
|
||||
// Guards that the #1565 fix buckets the error as ct, not as always-retryable.
|
||||
RetryMockWebSocket.clearScenarios();
|
||||
|
||||
const originalUrl = 'ws://localhost:3000#rc=2&rp=4xx';
|
||||
const cleanUrl = 'ws://localhost:3000';
|
||||
RetryMockWebSocket.setUrlMapping(cleanUrl, originalUrl);
|
||||
|
||||
const retryScenario = {
|
||||
attempts: [
|
||||
{ type: 'handshake-timeout' }
|
||||
]
|
||||
};
|
||||
RetryMockWebSocket.setRetryScenario('rc=2&rp=4xx', retryScenario);
|
||||
|
||||
const hook = {
|
||||
url: originalUrl,
|
||||
username: 'username',
|
||||
password: 'password'
|
||||
};
|
||||
|
||||
const params = {
|
||||
callSid: 'test_handshake_timeout_no_retry'
|
||||
};
|
||||
|
||||
// WHEN & THEN
|
||||
const requestor = new WsRequestor(logger, "account_sid", hook, "webhook_secret");
|
||||
try {
|
||||
await requestor.request('session:new', hook, params, {});
|
||||
t.fail('Should have thrown an error');
|
||||
} catch (err) {
|
||||
const errorMessage = err.message || err.toString() || String(err);
|
||||
t.ok(errorMessage.includes(HANDSHAKE_TIMEOUT_MESSAGE),
|
||||
'ws properly failed without retry for handshake timeout when rp=4xx');
|
||||
t.equal(RetryMockWebSocket.getConnectionAttempts('rc=2&rp=4xx'), 1,
|
||||
'should have made only 1 connection attempt');
|
||||
t.end();
|
||||
}
|
||||
});
|
||||
|
||||
test('WS Retry - default behavior (no hash params) should use ct policy', async (t) => {
|
||||
// GIVEN
|
||||
RetryMockWebSocket.clearScenarios();
|
||||
|
||||
Reference in New Issue
Block a user