Cloud-Automation / node-modbus

Modbus TCP Client/Server implementation for Node.JS
456 stars 169 forks source link

Client currentRequest promise is not resolving on ECONNRESET #305

Closed jla closed 2 years ago

jla commented 2 years ago

Hello, first of all, thanks for this wonderful library.

I'm experiencing some issues on an unreliable network connection, sometimes the socket receives an ECONNRESET, just after the write, and then socket is closed.

The onClose event handler clears the timeout on the current request, but as the socket is closed, no further data is received so the request is not resolved/rejected.

Is a corner case not easily replicable on the field, this is a test that tries to emulate this behaviour, on the current code, it timeouts:


    it.only('should handle a valid request with read ECONNRESET', function (done) {
      const socket = new EventEmitter();
      socket.write  = function() {
        // Socket receives a read ECONNRESET, the socket is closed and no further data is received
        socket.emit('close');
      }
      const client = new Modbus.client.TCP(socket, 2, 100) // unit id = 2, timeout = 100ms
      socket.emit('connect')

      client.readCoils(10, 11)
        .then(function (resp) {
          assert.ok(false)
        }).catch(function (e) {
          assert.equal('Offline', e.err)
          done()
        })
    })

A possible solution (I'm sure there is a better one) is to force the rejection of the currentRequest on the onClose event handler:

export default abstract class MBClientRequestHandler<S extends Stream.Duplex, Req extends ModbusAbstractRequest> {

  protected _onClose () {
    this._state = 'offline'
    this._currentRequest && this._currentRequest.reject(new UserRequestError({
      err: OFFLINE,
      message: 'connection to modbus server closed'
    }))
    this._clearAllRequests()
  }

}
stefanpoeter commented 2 years ago

Hey @jla, I think that is a valid point. Would to like to put this in a pull request?