Tests: fix flake/race in setOptionsCallbackBehaviour caused by protoc…

…ol v2
ably · Jan 17, 2023 · 1bf3e44 · 1bf3e44
1 parent 0a9770b
commit 1bf3e44
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 12 deletions.
diff --git a/src/common/lib/client/realtimechannel.ts b/src/common/lib/client/realtimechannel.ts
@@ -161,17 +161,23 @@ class RealtimeChannel extends Channel {
        * rejecting messages until we have confirmation that the options have changed,
        * which would unnecessarily lose message continuity. */
       this.attachImpl();
-      this._allChannelChanges.once(function (this: { event: string }, stateChange: ConnectionStateChange) {
-        switch (this.event) {
-          case 'update':
-          case 'attached':
-            _callback?.(null);
-            return;
-          default:
-            _callback?.(stateChange.reason);
-            return;
+      // Ignore 'attaching' -- could be just due to to a resume & reattach, should not
+      // call back setOptions until we're definitely attached with the new options (or
+      // else in a terminal state)
+      this._allChannelChanges.once(
+        ['attached', 'update', 'detached', 'failed'],
+        function (this: { event: string }, stateChange: ConnectionStateChange) {
+          switch (this.event) {
+            case 'update':
+            case 'attached':
+              _callback?.(null);
+              return;
+            default:
+              _callback?.(stateChange.reason);
+              return;
+          }
         }
-      });
+      );
     } else {
       _callback();
     }

diff --git a/test/realtime/channel.test.js b/test/realtime/channel.test.js
@@ -708,7 +708,10 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async
                 },
                 function (cb) {
                   var channelUpdated = false;
-                  channel._allChannelChanges.on('update', function () {
+                  // attached or update: in case this is happening in parallel with
+                  // a transport upgrade, we might flip to attaching, meaning it'll come
+                  // through as attached not update
+                  channel._allChannelChanges.on(['attached', 'update'], function () {
                     channelUpdated = true;
                   });
 
@@ -737,7 +740,7 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async
                 },
                 function (cb) {
                   var channelUpdated = false;
-                  channel._allChannelChanges.on('update', function () {
+                  channel._allChannelChanges.on(['attached', 'update'], function () {
                     channelUpdated = true;
                   });