Bump websocket connect timeout #382
Conversation
NPM Publishing labels 🏷️🟢 This PR has valid version labels and will cause a |
akuzni2
force-pushed
the
websocket-metric
branch
from
a8ee5a5 to
a7c2b7f
Compare
| @@ -157,7 +157,7 @@ export class WebSocketTransport< | |||
| return { | |||
| // Called when the WS connection is opened | |||
| open: async (event: WebSocket.Event) => { | |||
| logger.debug(`Opened websocket connection. (event type ${event.type})`) | |||
| logger.info(`Opened websocket connection. (event type ${event.type})`) | |||
There was a problem hiding this comment.
This is an important type of event that is good to set at info level since it should be infrequent and manages the entire stream of data
There was a problem hiding this comment.
there are certain EAs (eg: coinmetrics) that require reconnection to change the subscription set. I'm not sure this is the correct approach. Maybe in the specific EA if you find this valuable to log at info, stick a separate log in the specific open handler?
There was a problem hiding this comment.
That's a good suggestion - will implement locally
| @@ -246,7 +246,7 @@ export const BaseSettingsDefinition = { | |||
| 'The maximum amount of time in milliseconds to wait for the websocket connection to open (including custom open handler)', | |||
| type: 'number', | |||
| default: 10_000, | |||
| validate: validator.integer({ min: 500, max: 30_000 }), | |||
| validate: validator.integer({ min: 500, max: 180000 }), | |||
There was a problem hiding this comment.
not opposed to this per se, but also not sure this is correct, discussion: https://chainlink-core.slack.com/archives/C02FM5ZP2TC/p1738019106947369
We need to be able to adjust the timeout level to be longer if some EA's are under heavy load we can still connect without doing multiple re-attempts