kubernetes-client icon indicating copy to clipboard operation
kubernetes-client copied to clipboard

Stream errors are silently dropped

Open Stono opened this issue 5 years ago • 1 comments

Hi, We're trying to detect when a watch event has timed out. In our use case, this is typically because the kubernetes master has restarted or been upgraded. It is quite easily simulated by setting up a watch, killing your wifi for a couple of minutes, then reconnecting. You'll see the watch doesn't timeout and the stream is still readable.

Setting the timeout property on request will use the setting during a stream to decide if it is dead (see https://github.com/request/request/blob/df346d8531ac4b8c360df301f228d5767d0e374e/request.js#L807):

const backend = new Request({ kubeconfig, request: { timeout: 60000 } });

However setting that property just leads to kubernetes-client silently existing the watch, with no way to capture the timeout.

I believe the issue is here: https://github.com/godaddy/kubernetes-client/blob/master/backends/request/client.js#L225

If you change that to:

if (options.stream) return request(requestOptions, (err) => { console.log(err); }) 

You see the timeout:

Error: ESOCKETTIMEDOUT
    at ClientRequest.<anonymous> (/Users/karl.stoney/git/autotrader/node-at-kubernetes/node_modules/request/request.js:816:19)
    at Object.onceWrapper (events.js:299:28)
    at ClientRequest.emit (events.js:210:5)
    at ClientRequest.EventEmitter.emit (domain.js:476:20)
    at TLSSocket.emitRequestTimeout (_http_client.js:690:9)
    at Object.onceWrapper (events.js:299:28)
    at TLSSocket.emit (events.js:210:5)
    at TLSSocket.EventEmitter.emit (domain.js:476:20)
    at TLSSocket.Socket._onTimeout (net.js:468:8)
    at listOnTimeout (internal/timers.js:531:17) {
  code: 'ESOCKETTIMEDOUT',
  connect: false
}

I believe this function should probably return a Promise, like the line below, and throw this error when it happens so that the consumer can get it, and reconnect?

In other news request is deprecated.

Stono avatar May 20 '20 13:05 Stono

It seems like pump isn't passing through the error, if you add:

  async getWatchObjectStream (options) {
    console.log('getting watch stream')
    const jsonStream = new JSONStream()
    const stream = this.http(Object.assign({ stream: true }, options))
    // pass the error through
    stream.on('error', (err) => { jsonStream.emit('error', err) })
    pump(stream, jsonStream)
    return jsonStream
  }

we're able to catch it in the client

Stono avatar May 20 '20 13:05 Stono