Rapsssito / react-native-tcp-socket

React Native TCP socket API for Android, iOS & macOS with SSL/TLS support.
MIT License
303 stars 81 forks source link

Incorrect implementation of .setTimeout - with fix #194

Open Fabian-Lobnig opened 3 weeks ago

Fabian-Lobnig commented 3 weeks ago

Description

Your .setTimeout method does not work correctly - when calling .setTimeout the "timeout" event is only called once Also your .setTimeout only takes write activity into account The NodeJS implementation however states "timeout" should be called after ms of any inactivity

Steps to reproduce

Steps to reproduce the behavior:

  1. Create TCP Server
  2. Create TCP connection
  3. Call .setTimeout(3000)
  4. Don`t send anything
  5. Received "timeout" after 3000ms as expected
  6. Send something(timeout is not reset)
  7. Wait another 3000ms --> No "timeout" event will be received

Fix

@@ -182,10 +182,11 @@ export default class Socket extends EventEmitter {
      * @param {() => void} [callback]
      */
     setTimeout(timeout, callback) {
-        if (timeout === 0) {
+        this._timeoutMsecs = timeout;
+        if (this._timeoutMsecs === 0) {
             this._clearTimeout();
         } else {
-            this._activateTimer(timeout);
+            this._resetTimeout();
         }
         if (callback) this.once('timeout', callback);
         return this;
@@ -193,15 +194,15 @@ export default class Socket extends EventEmitter {

     /**
      * @private
-     * @param {number} [timeout]
      */
-    _activateTimer(timeout) {
-        if (timeout !== undefined) this._timeoutMsecs = timeout;
-        this._clearTimeout();
-        this._timeout = setTimeout(() => {
+    _resetTimeout() {
+        if (this._timeoutMsecs !== 0) {
             this._clearTimeout();
-            this.emit('timeout');
-        }, this._timeoutMsecs);
+            this._timeout = setTimeout(() => {
+                this._clearTimeout();
+                this.emit('timeout');
+            }, this._timeoutMsecs);
+        }
     }

     /**
@@ -327,7 +328,6 @@ export default class Socket extends EventEmitter {
      * @return {boolean}
      */
     write(buffer, encoding, cb) {
-        const self = this;
         if (this._pending || this._destroyed) throw new Error('Socket is closed.');

         const generatedBuffer = this._generateSendBuffer(buffer, encoding);
@@ -340,7 +340,7 @@ export default class Socket extends EventEmitter {
                 this._msgEvtEmitter.removeListener('written', msgEvtHandler);
                 this._writeBufferSize -= generatedBuffer.byteLength;
                 this._lastRcvMsgId = msgId;
-                if (self._timeout) self._activateTimer();
+                this._resetTimeout();
                 if (this.writableNeedDrain && this._lastSentMsgId === msgId) {
                     this.writableNeedDrain = false;
                     this.emit('drain');
@@ -434,6 +434,7 @@ export default class Socket extends EventEmitter {
      */
     _onDeviceDataEvt = (/** @type {{ id: number; data: string; }} */ evt) => {
         if (evt.id !== this._id) return;
+        this._resetTimeout();
         if (!this._paused) {
             const bufferData = Buffer.from(evt.data, 'base64');
             this._bytesRead += bufferData.byteLength;

Expected

Receive "timeout" event after ms of any inactivity

Relevant information

| OS | any - this is js code | | react-native-tcp-socket | 6.0.6 |

Rapsssito commented 3 weeks ago

@Fabian-Lobnig, thanks for the feedback! Could you create a PR with your fix?

Fabian-Lobnig commented 2 weeks ago

Hi,I ll take some time because I m very busy atmThat is why I added the fix diff into the issue, maybe someone gets to it faster than I will but I will try to make it happenOn Jun 19, 2024 14:44, Rodrigo Martín @.***> wrote: @Fabian-Lobnig, thanks for the feedback! Could you create a PR with your fix?

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>