Timeout callback

[glenarama]

Utilising a TLS connection i don't appear to get any timeout callback/error. Looking at the code, I don't on timeout and wondered if/how it was handled?

Thanks!

[Bugs5382]

@glenarama - Let me check.... I am pretty sure I had to take out the timeout code or option since tls.connect does this by default. I had to remove and move around how things work. The only difference between the TLS and non-TLS connect is just it having to use certificates to connect and the timeout on non-TLS does work.

[glenarama]

Im ok with the default timeout actually - but I can't seem to capture the timeout event. It isn't thrown as an error.

[Bugs5382]

The timeout event has been elusive to me as well... I think I had to get rid of it cause the timeout happens and then the error event kicked in. Originally I was overriding "error". emit on the socket, causing some issues that you reported before so I had to re-write it to no folow that format....

Use the:

 this.emit('client.error', connectionError)

Event. It should capture a timeout.

[glenarama]

Do we need something like the below on the connection client to throw it?

client.on('timeout', function timeout() {
    callback(new Error('Request timed out'));
  });

[Bugs5382]

I had that at one point... didn't quite seem to do anything cause by the time i try to do a "retry", timeout event is not thrown. I think that might be the issue cause I am forcing the socket to skip that in a way. I can see if putting it back in works and through it against the server (which I created a docker instance to do testing and the corepoint server) to see if it throws, but since I built int he auto-reconnect, it might not fire.

[Bugs5382]

Yeah, so..

Event: 'timeout'# Added in: v0.1.90 Emitted if the socket times out from inactivity. This is only to notify that the socket has been idle. The user must manually close the connection.

See also: socket.setTimeout().

This is from Node Docs. Timeout is idle, not connection. I had a connectionTimer in there but it was being messed up. I can see if I can add it back end to send client.timeout event vs. 'timeout'

[Bugs5382]

So I could do this:

But it just be continuous 'timeout' emits until it connects. I rather have a limit as well of the number of attempts before moving along. Pairing that with the code underneath, I would skip this block if there was already a successful connection prior until the max retires already complicating the situation. So unlimited timeouts which then the app would have to manage and retry at that point or it's own internal count of when it fals completely.

[glenarama]

Yes ideal workflow for me would be:

Connect -> [Timeout OR Error] -> Retry until count -> Emit Timeout or Error message -> Exit

[Bugs5382]

Ok. I think the min. timeout will have to be 10 seconds cause in 99.6% of the time connection will happen right away. This way as well I can create a proper unit test without having to override the defaults.

[Bugs5382]

@glenarama Ok.. check this out updated unit test:

From adding this:

That will fire 'client.timeout' in this case in about 1 second. The console would be gone, but ECONNREFUSED which was already being captures in this unit test from client.error works. client.timeout just says, yep, we tried again and if you wanted the error, check on client.error event for what really happened.

[Bugs5382]

Fixed in #83